From ac6ed4687f5934cee8fba83977c2d1038e2a97ae Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 19:37:19 -0500 Subject: [PATCH 01/21] PYTHON-6040 Preserve 1:1 index correspondence in client metadata --- pymongo/driver_info.py | 2 + pymongo/pool_options.py | 91 +++++++++++++++++++++++++++-------------- 2 files changed, 62 insertions(+), 31 deletions(-) diff --git a/pymongo/driver_info.py b/pymongo/driver_info.py index 18a51ae638..d4c444ce44 100644 --- a/pymongo/driver_info.py +++ b/pymongo/driver_info.py @@ -42,5 +42,7 @@ def __new__( raise TypeError( f"Wrong type for DriverInfo {key} option, value must be an instance of str, not {type(value)}" ) + if value and "|" in value: + raise ValueError(f"DriverInfo {key} must not contain the '|' delimiter") return self diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 8b26b4baf2..68d24ef61e 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -235,25 +235,51 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: if encoded_size <= _MAX_METADATA_SIZE: return # 5. Truncate driver info. - overflow = encoded_size - _MAX_METADATA_SIZE driver = metadata.get("driver", {}) if driver: - # Truncate driver version. - driver_version = driver.get("version")[:-overflow] - if len(driver_version) >= len(_METADATA["driver"]["version"]): - metadata["driver"]["version"] = driver_version - else: - metadata["driver"]["version"] = _METADATA["driver"]["version"] - encoded_size = len(bson.encode(metadata)) - if encoded_size <= _MAX_METADATA_SIZE: - return - # Truncate driver name. - overflow = encoded_size - _MAX_METADATA_SIZE - driver_name = driver.get("name")[:-overflow] - if len(driver_name) >= len(_METADATA["driver"]["name"]): - metadata["driver"]["name"] = driver_name - else: - metadata["driver"]["name"] = _METADATA["driver"]["name"] + # Truncate the driver name and version in lockstep so that the + # pipe-delimited name and version entries remain index-aligned (1:1). + # Trimming never removes a delimiter from one side alone, so the number + # of "|" in the name always matches the version. Under a large overflow + # the appended |c / |async / wrapped-driver entries are dropped together. + while True: + encoded_size = len(bson.encode(metadata)) + if encoded_size <= _MAX_METADATA_SIZE: + break + overflow = encoded_size - _MAX_METADATA_SIZE + previous = (metadata["driver"].get("name"), metadata["driver"].get("version")) + + name = metadata["driver"].get("name", "") + if len(name) > len(_METADATA["driver"]["name"]): + # Trim the tail of the name, never dropping below the base name. + name = name[:-overflow] + if len(name) < len(_METADATA["driver"]["name"]): + name = _METADATA["driver"]["name"] + metadata["driver"]["name"] = name + else: + # Name is already minimal; trim the version's trailing content. + # Trimming to "" is fine: keeping the delimiter preserves the + # index alignment. + parts = metadata["driver"].get("version", "").split("|") + if len(parts) > 1: + last = parts[-1] + parts[-1] = last[:-overflow] + metadata["driver"]["version"] = "|".join(parts) + else: + break + + # Rebuild the version to match the name's delimiter count so the + # entries stay 1:1 aligned. + parts = metadata["driver"].get("version", "").split("|") + target = metadata["driver"]["name"].count("|") + 1 + if len(parts) > target: + parts = parts[:target] + elif len(parts) < target: + parts += [""] * (target - len(parts)) + metadata["driver"]["version"] = "|".join(parts) + + if previous == (metadata["driver"].get("name"), metadata["driver"].get("version")): + break # If the first getaddrinfo call of this interpreter's life is on a thread, @@ -277,6 +303,7 @@ class PoolOptions: """ __slots__ = ( + "__appended_drivers", "__appname", "__compression_settings", "__connect_timeout", @@ -336,6 +363,7 @@ def __init__( self.__load_balanced = load_balanced self.__credentials = credentials self.__metadata = copy.deepcopy(_METADATA) + self.__appended_drivers: list[DriverInfo] = [] if appname: self.__metadata["application"] = {"name": appname} @@ -353,11 +381,19 @@ def __init__( self.__metadata["driver"]["name"], "c", ) + self.__metadata["driver"]["version"] = "{}|{}".format( + self.__metadata["driver"]["version"], + "", + ) if not is_sync: self.__metadata["driver"]["name"] = "{}|{}".format( self.__metadata["driver"]["name"], "async", ) + self.__metadata["driver"]["version"] = "{}|{}".format( + self.__metadata["driver"]["version"], + "", + ) if driver: self._update_metadata(driver) @@ -368,28 +404,21 @@ def __init__( _truncate_metadata(self.__metadata) def _update_metadata(self, driver: DriverInfo) -> None: - """Updates the client's metadata""" - if driver.name and driver.name.lower() in self.__metadata["driver"]["name"].lower().split( - "|" - ): + """Updates the client's metadata.""" + if driver in self.__appended_drivers: return metadata = copy.deepcopy(self.__metadata) - if driver.name: - metadata["driver"]["name"] = "{}|{}".format( - metadata["driver"]["name"], - driver.name, - ) - if driver.version: - metadata["driver"]["version"] = "{}|{}".format( - metadata["driver"]["version"], - driver.version, - ) + metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name or "") + metadata["driver"]["version"] = "{}|{}".format( + metadata["driver"]["version"], driver.version or "" + ) if driver.platform: metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) self.__metadata = metadata + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: From 5f2e74cc269878f43e8cbf13d3d9db055cd1728f Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 19:37:24 -0500 Subject: [PATCH 02/21] PYTHON-6040 Update tests for index-aligned client metadata --- test/asynchronous/test_client.py | 40 +++++++- test/asynchronous/test_client_metadata.py | 114 +++++++++++++++++++--- test/mockupdb/test_handshake.py | 4 +- test/test_client.py | 40 +++++++- test/test_client_metadata.py | 114 +++++++++++++++++++--- 5 files changed, 277 insertions(+), 35 deletions(-) diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 90a2d33a45..e9d426c045 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -134,6 +134,19 @@ _IS_SYNC = False +def _driver_version(base_version: str, name: str, last_version: str | None = None) -> str: + """Build a metadata driver version aligned 1:1 with ``name`` segments. + + The ``|c`` and ``|async`` name segments always have an empty version entry, + so the version string has one delimiter per name delimiter. ``last_version`` + is used when the final segment carries a wrapped driver's version. + """ + segments = [""] * name.count("|") + if last_version is not None: + segments[-1] = last_version + return "|".join([base_version, *segments]) + + class AsyncClientUnitTest(AsyncUnitTest): """AsyncMongoClient tests that don't require a server.""" @@ -386,6 +399,9 @@ async def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c|async" else: metadata["driver"]["name"] = "PyMongo|async" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) metadata["application"] = {"name": "foobar"} client = self.simple_client("mongodb://foo:27017/?appname=foobar&connect=false") options = client.options @@ -412,7 +428,9 @@ async def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c|async|FooDriver" else: metadata["driver"]["name"] = "PyMongo|async|FooDriver" - metadata["driver"]["version"] = "{}|1.2.3".format(_METADATA["driver"]["version"]) + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) client = self.simple_client( "foo", 27017, @@ -422,6 +440,13 @@ async def test_metadata(self): ) options = client.options self.assertEqual(options.pool_options.metadata, metadata) + if has_c(): + metadata["driver"]["name"] = "PyMongo|c|async|FooDriver" + else: + metadata["driver"]["name"] = "PyMongo|async|FooDriver" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) metadata["platform"] = "{}|FooPlatform".format(_METADATA["platform"]) client = self.simple_client( "foo", @@ -438,19 +463,29 @@ async def test_metadata(self): connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) client = self.simple_client( driver=DriverInfo(name="s" * _MAX_METADATA_SIZE, version="s" * _MAX_METADATA_SIZE), connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): @@ -2224,6 +2259,9 @@ async def _test_handshake(self, env_vars, expected_env): metadata["driver"]["name"] = "PyMongo|c|async" else: metadata["driver"]["name"] = "PyMongo|async" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) if expected_env is not None: metadata["env"] = expected_env diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 1a07e835a8..4615317c5b 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -99,20 +99,14 @@ async def check_metadata_added( new_name, new_version, new_platform, new_metadata = await self.send_ping_and_get_metadata( client, True ) - if add_name is not None and add_name.lower() in name.lower().split("|"): - self.assertEqual(name, new_name) - self.assertEqual(version, new_version) - self.assertEqual(platform, new_platform) - else: - self.assertEqual(new_name, f"{name}|{add_name}" if add_name is not None else name) - self.assertEqual( - new_version, - f"{version}|{add_version}" if add_version is not None else version, - ) - self.assertEqual( - new_platform, - f"{platform}|{add_platform}" if add_platform is not None else platform, - ) + # Name and version always get a delimiter (empty string if None) to + # preserve 1:1 index correspondence. + self.assertEqual(new_name, f"{name}|{add_name or ''}") + self.assertEqual(new_version, f"{version}|{add_version or ''}") + self.assertEqual( + new_platform, + f"{platform}|{add_platform}" if add_platform is not None else platform, + ) metadata.pop("driver") metadata.pop("platform") @@ -216,8 +210,42 @@ async def test_duplicate_driver_name_no_op(self): await self.check_metadata_added(client, "framework", None, None) # wait for connection to become idle await asyncio.sleep(0.005) - # add same metadata again - await self.check_metadata_added(client, "Framework", None, None) + # Append the exact same DriverInfo again: no-op. + name, version, platform, _ = await self.send_ping_and_get_metadata(client, True) + await asyncio.sleep(0.005) + client.append_metadata(DriverInfo("framework", None, None)) + new_name, new_version, new_platform, _ = await self.send_ping_and_get_metadata(client, True) + self.assertEqual(new_name, name) + self.assertEqual(new_version, version) + self.assertEqual(new_platform, platform) + + async def test_append_metadata_rejects_delimiter(self): + cases = [ + ("frame|work", "2.0", "Framework Platform"), + ("framework", "2|0", "Framework Platform"), + ("framework", "2.0", "Framework|Platform"), + ] + for name, version, platform in cases: + with self.subTest(name=name, version=version, platform=platform): + client = await self.async_rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + # Send initial handshake. + name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( + client, True + ) + await asyncio.sleep(0.005) + # Appending metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = await self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + await client.close() async def test_handshake_documents_include_backpressure(self): # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of @@ -232,6 +260,60 @@ async def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") + async def test_index_correspondence(self): + cases = [ + ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), + ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), + ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), + ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), + ( + "Non-adjacent duplicate", + [("F1", "1.0"), ("F2", "2.0"), ("F1", "1.0")], + "|F1|F2", + "|1.0|2.0", + ), + ( + "Platform-only difference is not a duplicate", + [("F1", "1.0", "P1"), ("F1", "1.0", "P2")], + "|F1|F1", + "|1.0|1.0", + ), + ("Wrapper matching the driver's own identity", [("PyMongo", None)], "|PyMongo", "|"), + ] + for ( + description, + appended, + expected_name_suffix, + expected_version_suffix, + ) in cases: + with self.subTest(description=description): + client = await self.async_rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True) + await asyncio.sleep(0.005) + + # Append each DriverInfoOptions in order. + for opts in appended: + d_name = opts[0] if len(opts) > 0 else None + assert d_name is not None + d_version = opts[1] if len(opts) > 1 else None + d_platform = opts[2] if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name, d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) + + assert name0 is not None + assert version0 is not None + self.assertEqual(name1, name0 + expected_name_suffix) + self.assertEqual(version1, version0 + expected_version_suffix) + await client.close() + if __name__ == "__main__": unittest.main() diff --git a/test/mockupdb/test_handshake.py b/test/mockupdb/test_handshake.py index 2772e6f77a..e3a3fc0562 100644 --- a/test/mockupdb/test_handshake.py +++ b/test/mockupdb/test_handshake.py @@ -49,9 +49,11 @@ def _check_handshake_data(request): assert data["application"] == {"name": "my app"} if has_c(): name = "PyMongo|c" + version = pymongo_version + "|" else: name = "PyMongo" - assert data["driver"] == {"name": name, "version": pymongo_version} + version = pymongo_version + assert data["driver"] == {"name": name, "version": version} # Keep it simple, just check these fields exist. assert "os" in data diff --git a/test/test_client.py b/test/test_client.py index 249f95d8fc..eb0022c9e8 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -133,6 +133,19 @@ _IS_SYNC = True +def _driver_version(base_version: str, name: str, last_version: str | None = None) -> str: + """Build a metadata driver version aligned 1:1 with ``name`` segments. + + The ``|c`` and ``|async`` name segments always have an empty version entry, + so the version string has one delimiter per name delimiter. ``last_version`` + is used when the final segment carries a wrapped driver's version. + """ + segments = [""] * name.count("|") + if last_version is not None: + segments[-1] = last_version + return "|".join([base_version, *segments]) + + class ClientUnitTest(UnitTest): """MongoClient tests that don't require a server.""" @@ -379,6 +392,9 @@ def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c" else: metadata["driver"]["name"] = "PyMongo" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) metadata["application"] = {"name": "foobar"} client = self.simple_client("mongodb://foo:27017/?appname=foobar&connect=false") options = client.options @@ -405,7 +421,9 @@ def test_metadata(self): metadata["driver"]["name"] = "PyMongo|c|FooDriver" else: metadata["driver"]["name"] = "PyMongo|FooDriver" - metadata["driver"]["version"] = "{}|1.2.3".format(_METADATA["driver"]["version"]) + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) client = self.simple_client( "foo", 27017, @@ -415,6 +433,13 @@ def test_metadata(self): ) options = client.options self.assertEqual(options.pool_options.metadata, metadata) + if has_c(): + metadata["driver"]["name"] = "PyMongo|c|FooDriver" + else: + metadata["driver"]["name"] = "PyMongo|FooDriver" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" + ) metadata["platform"] = "{}|FooPlatform".format(_METADATA["platform"]) client = self.simple_client( "foo", @@ -431,19 +456,29 @@ def test_metadata(self): connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) client = self.simple_client( driver=DriverInfo(name="s" * _MAX_METADATA_SIZE, version="s" * _MAX_METADATA_SIZE), connect=False, ) options = client.options + truncated = options.pool_options.metadata["driver"] self.assertLessEqual( len(bson.encode(options.pool_options.metadata)), _MAX_METADATA_SIZE, ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): @@ -2177,6 +2212,9 @@ def _test_handshake(self, env_vars, expected_env): metadata["driver"]["name"] = "PyMongo|c" else: metadata["driver"]["name"] = "PyMongo" + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"] + ) if expected_env is not None: metadata["env"] = expected_env diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index f5ec92f2f3..68b132646c 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -99,20 +99,14 @@ def check_metadata_added( new_name, new_version, new_platform, new_metadata = self.send_ping_and_get_metadata( client, True ) - if add_name is not None and add_name.lower() in name.lower().split("|"): - self.assertEqual(name, new_name) - self.assertEqual(version, new_version) - self.assertEqual(platform, new_platform) - else: - self.assertEqual(new_name, f"{name}|{add_name}" if add_name is not None else name) - self.assertEqual( - new_version, - f"{version}|{add_version}" if add_version is not None else version, - ) - self.assertEqual( - new_platform, - f"{platform}|{add_platform}" if add_platform is not None else platform, - ) + # Name and version always get a delimiter (empty string if None) to + # preserve 1:1 index correspondence. + self.assertEqual(new_name, f"{name}|{add_name or ''}") + self.assertEqual(new_version, f"{version}|{add_version or ''}") + self.assertEqual( + new_platform, + f"{platform}|{add_platform}" if add_platform is not None else platform, + ) metadata.pop("driver") metadata.pop("platform") @@ -216,8 +210,42 @@ def test_duplicate_driver_name_no_op(self): self.check_metadata_added(client, "framework", None, None) # wait for connection to become idle time.sleep(0.005) - # add same metadata again - self.check_metadata_added(client, "Framework", None, None) + # Append the exact same DriverInfo again: no-op. + name, version, platform, _ = self.send_ping_and_get_metadata(client, True) + time.sleep(0.005) + client.append_metadata(DriverInfo("framework", None, None)) + new_name, new_version, new_platform, _ = self.send_ping_and_get_metadata(client, True) + self.assertEqual(new_name, name) + self.assertEqual(new_version, version) + self.assertEqual(new_platform, platform) + + def test_append_metadata_rejects_delimiter(self): + cases = [ + ("frame|work", "2.0", "Framework Platform"), + ("framework", "2|0", "Framework Platform"), + ("framework", "2.0", "Framework|Platform"), + ] + for name, version, platform in cases: + with self.subTest(name=name, version=version, platform=platform): + client = self.rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + # Send initial handshake. + name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( + client, True + ) + time.sleep(0.005) + # Appending metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + client.close() def test_handshake_documents_include_backpressure(self): # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of @@ -232,6 +260,60 @@ def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") + def test_index_correspondence(self): + cases = [ + ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), + ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), + ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), + ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), + ( + "Non-adjacent duplicate", + [("F1", "1.0"), ("F2", "2.0"), ("F1", "1.0")], + "|F1|F2", + "|1.0|2.0", + ), + ( + "Platform-only difference is not a duplicate", + [("F1", "1.0", "P1"), ("F1", "1.0", "P2")], + "|F1|F1", + "|1.0|1.0", + ), + ("Wrapper matching the driver's own identity", [("PyMongo", None)], "|PyMongo", "|"), + ] + for ( + description, + appended, + expected_name_suffix, + expected_version_suffix, + ) in cases: + with self.subTest(description=description): + client = self.rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = self.send_ping_and_get_metadata(client, True) + time.sleep(0.005) + + # Append each DriverInfoOptions in order. + for opts in appended: + d_name = opts[0] if len(opts) > 0 else None + assert d_name is not None + d_version = opts[1] if len(opts) > 1 else None + d_platform = opts[2] if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name, d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) + + assert name0 is not None + assert version0 is not None + self.assertEqual(name1, name0 + expected_name_suffix) + self.assertEqual(version1, version0 + expected_version_suffix) + client.close() + if __name__ == "__main__": unittest.main() From b3d18d12f1c3ba51841abe23a686a98df143db2e Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 20:14:01 -0500 Subject: [PATCH 03/21] PYTHON-6040 Address PR review feedback Trim the _truncate_metadata comments and number/label the handshake prose tests (backpressure no. 9, delimiter no. 10, index no. 11). --- pymongo/pool_options.py | 9 ++----- test/asynchronous/test_client_metadata.py | 29 +++++++++++++---------- test/test_client_metadata.py | 29 +++++++++++++---------- 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 68d24ef61e..5a322d741a 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -237,11 +237,8 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: # 5. Truncate driver info. driver = metadata.get("driver", {}) if driver: - # Truncate the driver name and version in lockstep so that the - # pipe-delimited name and version entries remain index-aligned (1:1). - # Trimming never removes a delimiter from one side alone, so the number - # of "|" in the name always matches the version. Under a large overflow - # the appended |c / |async / wrapped-driver entries are dropped together. + # Truncate the driver name and version in lockstep so the pipe-delimited + # entries stay 1:1 index-aligned. while True: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: @@ -258,8 +255,6 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: metadata["driver"]["name"] = name else: # Name is already minimal; trim the version's trailing content. - # Trimming to "" is fine: keeping the delimiter preserves the - # index alignment. parts = metadata["driver"].get("version", "").split("|") if len(parts) > 1: last = parts[-1] diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 4615317c5b..e740140859 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -219,6 +219,21 @@ async def test_duplicate_driver_name_no_op(self): self.assertEqual(new_version, version) self.assertEqual(new_platform, platform) + # Prose test no. 9 + async def test_handshake_documents_include_backpressure(self): + # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of + # connection establishment. + client = await self.async_rs_or_single_client("mongodb://" + self.server.address_string) + + # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is + # established on all topologies. Note: MockupDB only supports standalone servers. + await client.admin.command("ping") + + # Assert that for every handshake document intercepted: + # the document has a field `backpressure` whose value is `"2"`. + self.assertEqual(self.handshake_req["backpressure"], "2") + + # Prose test no. 10 async def test_append_metadata_rejects_delimiter(self): cases = [ ("frame|work", "2.0", "Framework Platform"), @@ -247,19 +262,7 @@ async def test_append_metadata_rejects_delimiter(self): self.assertEqual(platform1, platform0) await client.close() - async def test_handshake_documents_include_backpressure(self): - # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of - # connection establishment. - client = await self.async_rs_or_single_client("mongodb://" + self.server.address_string) - - # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is - # established on all topologies. Note: MockupDB only supports standalone servers. - await client.admin.command("ping") - - # Assert that for every handshake document intercepted: - # the document has a field `backpressure` whose value is `"2"`. - self.assertEqual(self.handshake_req["backpressure"], "2") - + # Prose test no. 11 async def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 68b132646c..998353c8ba 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -219,6 +219,21 @@ def test_duplicate_driver_name_no_op(self): self.assertEqual(new_version, version) self.assertEqual(new_platform, platform) + # Prose test no. 9 + def test_handshake_documents_include_backpressure(self): + # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of + # connection establishment. + client = self.rs_or_single_client("mongodb://" + self.server.address_string) + + # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is + # established on all topologies. Note: MockupDB only supports standalone servers. + client.admin.command("ping") + + # Assert that for every handshake document intercepted: + # the document has a field `backpressure` whose value is `"2"`. + self.assertEqual(self.handshake_req["backpressure"], "2") + + # Prose test no. 10 def test_append_metadata_rejects_delimiter(self): cases = [ ("frame|work", "2.0", "Framework Platform"), @@ -247,19 +262,7 @@ def test_append_metadata_rejects_delimiter(self): self.assertEqual(platform1, platform0) client.close() - def test_handshake_documents_include_backpressure(self): - # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of - # connection establishment. - client = self.rs_or_single_client("mongodb://" + self.server.address_string) - - # Send a `ping` command to the server and verify that the command succeeds. This ensure that a connection is - # established on all topologies. Note: MockupDB only supports standalone servers. - client.admin.command("ping") - - # Assert that for every handshake document intercepted: - # the document has a field `backpressure` whose value is `"2"`. - self.assertEqual(self.handshake_req["backpressure"], "2") - + # Prose test no. 11 def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), From b035cfbbed58b2cde8135fc9b493e3528d2ea87a Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 20:29:25 -0500 Subject: [PATCH 04/21] PYTHON-6040 Address Copilot review feedback Reapply the 512-byte handshake limit after append_metadata, guard the check/update/record sequence with a lock for thread-safe clients, and document the reserved '|' delimiter on DriverInfo. --- pymongo/driver_info.py | 4 ++++ pymongo/pool_options.py | 30 +++++++++++++++++++----------- test/asynchronous/test_client.py | 15 +++++++++++++++ test/test_client.py | 15 +++++++++++++++ 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/pymongo/driver_info.py b/pymongo/driver_info.py index d4c444ce44..54905e8872 100644 --- a/pymongo/driver_info.py +++ b/pymongo/driver_info.py @@ -31,6 +31,10 @@ class DriverInfo(namedtuple("DriverInfo", ["name", "version", "platform"])): can add its own info to this log message. Initialize with three strings like 'MyDriver', '1.2.3', 'some platform info'. Any of these strings may be None to accept PyMongo's default. + + The ``|`` character is the reserved delimiter used to join appended + metadata, so it must not appear in any of the fields. A + :class:`ValueError` is raised if it does. """ def __new__( diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 5a322d741a..d8107e8017 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -23,6 +23,7 @@ import os import platform import sys +import threading from collections.abc import MutableMapping from pathlib import Path from typing import TYPE_CHECKING, Any, Optional @@ -310,6 +311,7 @@ class PoolOptions: "__max_idle_time_seconds", "__max_pool_size", "__metadata", + "__metadata_lock", "__min_pool_size", "__pause_enabled", "__server_api", @@ -359,6 +361,7 @@ def __init__( self.__credentials = credentials self.__metadata = copy.deepcopy(_METADATA) self.__appended_drivers: list[DriverInfo] = [] + self.__metadata_lock = threading.Lock() if appname: self.__metadata["application"] = {"name": appname} @@ -400,20 +403,25 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata.""" - if driver in self.__appended_drivers: - return + with self.__metadata_lock: + if driver in self.__appended_drivers: + return - metadata = copy.deepcopy(self.__metadata) + metadata = copy.deepcopy(self.__metadata) - metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name or "") - metadata["driver"]["version"] = "{}|{}".format( - metadata["driver"]["version"], driver.version or "" - ) - if driver.platform: - metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + metadata["driver"]["name"] = "{}|{}".format( + metadata["driver"]["name"], driver.name or "" + ) + metadata["driver"]["version"] = "{}|{}".format( + metadata["driver"]["version"], driver.version or "" + ) + if driver.platform: + metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + + _truncate_metadata(metadata) - self.__metadata = metadata - self.__appended_drivers.append(driver) + self.__metadata = metadata + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index e9d426c045..32ce6dc152 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -486,6 +486,21 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Successive appends must also stay within the limit and keep name and + # version index-aligned after truncation. + client = self.simple_client(connect=False) + for i in range(80): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + options = client.options + truncated = options.pool_options.metadata["driver"] + self.assertLessEqual( + len(bson.encode(options.pool_options.metadata)), + _MAX_METADATA_SIZE, + ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index eb0022c9e8..4f2c9e7a7d 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -479,6 +479,21 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Successive appends must also stay within the limit and keep name and + # version index-aligned after truncation. + client = self.simple_client(connect=False) + for i in range(80): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + options = client.options + truncated = options.pool_options.metadata["driver"] + self.assertLessEqual( + len(bson.encode(options.pool_options.metadata)), + _MAX_METADATA_SIZE, + ) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): From 61ba3a20552d4d632267d0abde302feed65d26f3 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 14 Sep 2026 20:36:45 -0500 Subject: [PATCH 05/21] PYTHON-6040 Use fork-aware lock for metadata updates Use _create_lock() so the metadata lock is registered with pymongo.lock and reset after a fork, avoiding a deadlock in the child process. --- pymongo/pool_options.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index d8107e8017..22ed73cc9d 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -23,7 +23,6 @@ import os import platform import sys -import threading from collections.abc import MutableMapping from pathlib import Path from typing import TYPE_CHECKING, Any, Optional @@ -38,6 +37,7 @@ WAIT_QUEUE_TIMEOUT, has_c, ) +from pymongo.lock import _create_lock if TYPE_CHECKING: from pymongo.auth_shared import MongoCredential @@ -361,7 +361,7 @@ def __init__( self.__credentials = credentials self.__metadata = copy.deepcopy(_METADATA) self.__appended_drivers: list[DriverInfo] = [] - self.__metadata_lock = threading.Lock() + self.__metadata_lock = _create_lock() if appname: self.__metadata["application"] = {"name": appname} From 7e863c9011d607359a2c5168cb3cdee1eecea531 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 05:20:32 -0500 Subject: [PATCH 06/21] PYTHON-6040 Fix truncation order and platform recreation Trim wrapper version content before dropping name/version segments so driver identity is preserved, and recreate the platform field when a platform append follows truncation that removed it. --- pymongo/pool_options.py | 57 ++++++++++------------- test/asynchronous/test_client_metadata.py | 7 ++- test/test_client_metadata.py | 7 ++- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 22ed73cc9d..b2a1d24f75 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -235,46 +235,34 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: return - # 5. Truncate driver info. + # 5. Truncate driver info, keeping name and version 1:1 index-aligned. driver = metadata.get("driver", {}) if driver: - # Truncate the driver name and version in lockstep so the pipe-delimited - # entries stay 1:1 index-aligned. + # Trim the trailing wrapper version's content first so the driver's + # name identity is preserved for as long as possible. Only when no + # version content remains to trim do we drop the last name/version + # segment pair; both actions keep the entries 1:1 aligned. while True: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: break overflow = encoded_size - _MAX_METADATA_SIZE - previous = (metadata["driver"].get("name"), metadata["driver"].get("version")) - - name = metadata["driver"].get("name", "") - if len(name) > len(_METADATA["driver"]["name"]): - # Trim the tail of the name, never dropping below the base name. - name = name[:-overflow] - if len(name) < len(_METADATA["driver"]["name"]): - name = _METADATA["driver"]["name"] - metadata["driver"]["name"] = name + previous = (driver.get("name"), driver.get("version")) + n_parts = driver.get("name", "").split("|") + v_parts = driver.get("version", "").split("|") + + if len(v_parts) > 1 and v_parts[-1]: + v_parts[-1] = v_parts[-1][:-overflow] + driver["version"] = "|".join(v_parts) + elif len(n_parts) > 1: + n_parts.pop() + v_parts.pop() + driver["name"] = "|".join(n_parts) + driver["version"] = "|".join(v_parts) else: - # Name is already minimal; trim the version's trailing content. - parts = metadata["driver"].get("version", "").split("|") - if len(parts) > 1: - last = parts[-1] - parts[-1] = last[:-overflow] - metadata["driver"]["version"] = "|".join(parts) - else: - break - - # Rebuild the version to match the name's delimiter count so the - # entries stay 1:1 aligned. - parts = metadata["driver"].get("version", "").split("|") - target = metadata["driver"]["name"].count("|") + 1 - if len(parts) > target: - parts = parts[:target] - elif len(parts) < target: - parts += [""] * (target - len(parts)) - metadata["driver"]["version"] = "|".join(parts) - - if previous == (metadata["driver"].get("name"), metadata["driver"].get("version")): + break + + if previous == (driver.get("name"), driver.get("version")): break @@ -416,7 +404,10 @@ def _update_metadata(self, driver: DriverInfo) -> None: metadata["driver"]["version"], driver.version or "" ) if driver.platform: - metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + if "platform" in metadata: + metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + else: + metadata["platform"] = driver.platform _truncate_metadata(metadata) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index e740140859..a723fca8a4 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -267,7 +267,12 @@ async def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ( + "Equal versions do not collapse", + [("F1", "1.0"), ("F2", "1.0")], + "|F1|F2", + "|1.0|1.0", + ), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 998353c8ba..6dc2c7f4af 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -267,7 +267,12 @@ def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), + ( + "Equal versions do not collapse", + [("F1", "1.0"), ("F2", "1.0")], + "|F1|F2", + "|1.0|1.0", + ), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), From d45d98752f27a7f5e210fc1b4df52d17915fb6af Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 05:38:41 -0500 Subject: [PATCH 07/21] PYTHON-6040 Reverse prose test case and trim truncation comment Revert the 'Equal versions do not collapse' prose test case to the specification and shorten the truncation comment. --- pymongo/pool_options.py | 6 ++---- test/asynchronous/test_client_metadata.py | 7 +------ test/test_client_metadata.py | 7 +------ 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index b2a1d24f75..46dcb96a90 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -238,10 +238,8 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: # 5. Truncate driver info, keeping name and version 1:1 index-aligned. driver = metadata.get("driver", {}) if driver: - # Trim the trailing wrapper version's content first so the driver's - # name identity is preserved for as long as possible. Only when no - # version content remains to trim do we drop the last name/version - # segment pair; both actions keep the entries 1:1 aligned. + # Trim wrapper version content first, dropping paired segments only as + # a last resort, so name and version stay 1:1 aligned. while True: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index a723fca8a4..e740140859 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -267,12 +267,7 @@ async def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ( - "Equal versions do not collapse", - [("F1", "1.0"), ("F2", "1.0")], - "|F1|F2", - "|1.0|1.0", - ), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 6dc2c7f4af..998353c8ba 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -267,12 +267,7 @@ def test_index_correspondence(self): cases = [ ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ( - "Equal versions do not collapse", - [("F1", "1.0"), ("F2", "1.0")], - "|F1|F2", - "|1.0|1.0", - ), + ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), From 0194f4742d6898f323d11bddaa95f3b090d0c023 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 06:08:28 -0500 Subject: [PATCH 08/21] PYTHON-6040 Bound appended-driver tracking Only record drivers that remain representable in the truncated metadata, so __appended_drivers cannot grow without bound and the dedup membership check stays fast. Add a regression test. --- pymongo/pool_options.py | 12 +++++++++++- test/asynchronous/test_client.py | 6 ++++++ test/test_client.py | 6 ++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 46dcb96a90..1187770086 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -410,7 +410,17 @@ def _update_metadata(self, driver: DriverInfo) -> None: _truncate_metadata(metadata) self.__metadata = metadata - self.__appended_drivers.append(driver) + + # Keep the dedup list bounded: a driver that truncation dropped + # from the published metadata can't be re-appended anyway. + if driver.name: + represented = metadata["driver"]["name"].split("|")[-1] == driver.name + elif driver.version: + represented = metadata["driver"]["version"].split("|")[-1] == driver.version + else: + represented = True + if represented: + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 32ce6dc152..9cf5b2a8c4 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -501,6 +501,12 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Truncated-away drivers must not be retained, so the dedup list stays + # bounded instead of growing one entry per append. + self.assertLess( + len(options.pool_options._PoolOptions__appended_drivers), + 80, + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index 4f2c9e7a7d..f9c5a839f2 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -494,6 +494,12 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # Truncated-away drivers must not be retained, so the dedup list stays + # bounded instead of growing one entry per append. + self.assertLess( + len(options.pool_options._PoolOptions__appended_drivers), + 80, + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): From b652b78cf32be0461e7f899af2f125a363d844e8 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 06:25:02 -0500 Subject: [PATCH 09/21] PYTHON-6040 Bound dedup tracking for platform-only appends Use the name delimiter count before/after the update to decide whether an appended pair survived truncation, instead of a name/version branch that always recorded platform-only (empty name/version) drivers. --- pymongo/pool_options.py | 14 ++++-------- test/asynchronous/test_client.py | 39 ++++++++++++++++++-------------- test/test_client.py | 39 ++++++++++++++++++-------------- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 1187770086..ce61c8e62b 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -393,6 +393,7 @@ def _update_metadata(self, driver: DriverInfo) -> None: if driver in self.__appended_drivers: return + name_delims = self.__metadata["driver"]["name"].count("|") metadata = copy.deepcopy(self.__metadata) metadata["driver"]["name"] = "{}|{}".format( @@ -411,15 +412,10 @@ def _update_metadata(self, driver: DriverInfo) -> None: self.__metadata = metadata - # Keep the dedup list bounded: a driver that truncation dropped - # from the published metadata can't be re-appended anyway. - if driver.name: - represented = metadata["driver"]["name"].split("|")[-1] == driver.name - elif driver.version: - represented = metadata["driver"]["version"].split("|")[-1] == driver.version - else: - represented = True - if represented: + # Only track drivers whose appended name/version pair survived + # truncation (i.e. the name gained a segment), so __appended_drivers + # stays bounded and the dedup membership check stays fast. + if metadata["driver"]["name"].count("|") > name_delims: self.__appended_drivers.append(driver) @property diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 9cf5b2a8c4..2a81e3188b 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -486,27 +486,32 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) - # Successive appends must also stay within the limit and keep name and - # version index-aligned after truncation. + # Successive appends must stay within the limit and keep name and + # version index-aligned after truncation. Once the metadata saturates, + # further appends must not grow the dedup tracking list. client = self.simple_client(connect=False) - for i in range(80): + for i in range(300): client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) - options = client.options - truncated = options.pool_options.metadata["driver"] - self.assertLessEqual( - len(bson.encode(options.pool_options.metadata)), - _MAX_METADATA_SIZE, - ) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) self.assertEqual( - truncated["name"].count("|"), - truncated["version"].count("|"), - ) - # Truncated-away drivers must not be retained, so the dedup list stays - # bounded instead of growing one entry per append. - self.assertLess( - len(options.pool_options._PoolOptions__appended_drivers), - 80, + pool.metadata["driver"]["name"].count("|"), + pool.metadata["driver"]["version"].count("|"), ) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + # Platform-only appends (empty name/version) stay bounded the same way. + client = self.simple_client(connect=False) + for i in range(300): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index f9c5a839f2..21217c8f34 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -479,27 +479,32 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) - # Successive appends must also stay within the limit and keep name and - # version index-aligned after truncation. + # Successive appends must stay within the limit and keep name and + # version index-aligned after truncation. Once the metadata saturates, + # further appends must not grow the dedup tracking list. client = self.simple_client(connect=False) - for i in range(80): + for i in range(300): client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) - options = client.options - truncated = options.pool_options.metadata["driver"] - self.assertLessEqual( - len(bson.encode(options.pool_options.metadata)), - _MAX_METADATA_SIZE, - ) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) self.assertEqual( - truncated["name"].count("|"), - truncated["version"].count("|"), - ) - # Truncated-away drivers must not be retained, so the dedup list stays - # bounded instead of growing one entry per append. - self.assertLess( - len(options.pool_options._PoolOptions__appended_drivers), - 80, + pool.metadata["driver"]["name"].count("|"), + pool.metadata["driver"]["version"].count("|"), ) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name=f"D{i}", version=f"1.{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + # Platform-only appends (empty name/version) stay bounded the same way. + client = self.simple_client(connect=False) + for i in range(300): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + pool = client.options.pool_options + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) + count = len(pool._PoolOptions__appended_drivers) + for i in range(300, 600): + client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) + self.assertEqual(len(pool._PoolOptions__appended_drivers), count) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): From a016133f851a5d551ca063105a9fcd77dbdf7c08 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 07:28:03 -0500 Subject: [PATCH 10/21] PYTHON-6040 Cover empty-name entries in index correspondence Add 'Gap in middle (name)' and 'All names absent' cases and drop the non-None name assertion so empty name segments are verified to stay index-aligned. --- test/asynchronous/test_client_metadata.py | 5 +++-- test/test_client_metadata.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index e740140859..8bc67d713c 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -271,6 +271,8 @@ async def test_index_correspondence(self): ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), + ("Gap in middle (name)", [(None, "1.0"), ("F2", "2.0")], "||F2", "|1.0|2.0"), + ("All names absent", [(None, "1.0"), (None, "2.0")], "||", "|1.0|2.0"), ( "Non-adjacent duplicate", [("F1", "1.0"), ("F2", "2.0"), ("F1", "1.0")], @@ -303,10 +305,9 @@ async def test_index_correspondence(self): # Append each DriverInfoOptions in order. for opts in appended: d_name = opts[0] if len(opts) > 0 else None - assert d_name is not None d_version = opts[1] if len(opts) > 1 else None d_platform = opts[2] if len(opts) > 2 else None - client.append_metadata(DriverInfo(d_name, d_version, d_platform)) + client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) # New handshake with the appended metadata. name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 998353c8ba..07332a67c0 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -271,6 +271,8 @@ def test_index_correspondence(self): ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), + ("Gap in middle (name)", [(None, "1.0"), ("F2", "2.0")], "||F2", "|1.0|2.0"), + ("All names absent", [(None, "1.0"), (None, "2.0")], "||", "|1.0|2.0"), ( "Non-adjacent duplicate", [("F1", "1.0"), ("F2", "2.0"), ("F1", "1.0")], @@ -303,10 +305,9 @@ def test_index_correspondence(self): # Append each DriverInfoOptions in order. for opts in appended: d_name = opts[0] if len(opts) > 0 else None - assert d_name is not None d_version = opts[1] if len(opts) > 1 else None d_platform = opts[2] if len(opts) > 2 else None - client.append_metadata(DriverInfo(d_name, d_version, d_platform)) + client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) # New handshake with the appended metadata. name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) From b50cb0f3f53986f98ef44bbbf7135ca560af5d9b Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 07:45:03 -0500 Subject: [PATCH 11/21] PYTHON-6040 Align index-correspondence prose test to specifications Number index correspondence as prose test 10 and delimiter rejection as prose test 11, and mirror the specifications PR #1975 case table (order and content, resolving / at runtime). --- test/asynchronous/test_client_metadata.py | 61 ++++++++++++++++++----- test/test_client_metadata.py | 61 ++++++++++++++++++----- 2 files changed, 96 insertions(+), 26 deletions(-) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 8bc67d713c..e7292c299a 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -233,7 +233,7 @@ async def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") - # Prose test no. 10 + # Prose test no. 11 async def test_append_metadata_rejects_delimiter(self): cases = [ ("frame|work", "2.0", "Framework Platform"), @@ -262,16 +262,26 @@ async def test_append_metadata_rejects_delimiter(self): self.assertEqual(platform1, platform0) await client.close() - # Prose test no. 11 + # Prose test no. 10 async def test_index_correspondence(self): cases = [ + ("Gap in middle (name)", [(None, None), ("F2", None)], "||F2", "||"), ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), - ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), + ( + "Equal versions do not collapse", + [("F1", "{driver_version}")], + "|F1", + "|{driver_version}", + ), + ( + "Equal names do not collapse", + [("{driver_name}", "1.0")], + "|{driver_name}", + "|1.0", + ), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), - ("Gap in middle (name)", [(None, "1.0"), ("F2", "2.0")], "||F2", "|1.0|2.0"), ("All names absent", [(None, "1.0"), (None, "2.0")], "||", "|1.0|2.0"), ( "Non-adjacent duplicate", @@ -285,7 +295,12 @@ async def test_index_correspondence(self): "|F1|F1", "|1.0|1.0", ), - ("Wrapper matching the driver's own identity", [("PyMongo", None)], "|PyMongo", "|"), + ( + "Wrapper matching the driver's own identity", + [("{driver_name}", "{driver_version}")], + "|{driver_name}", + "|{driver_version}", + ), ] for ( description, @@ -302,20 +317,40 @@ async def test_index_correspondence(self): name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True) await asyncio.sleep(0.005) + assert name0 is not None + assert version0 is not None + driver_name = name0.split("|")[0] + driver_version = version0.split("|")[0] + + def resolve(value: Optional[str]) -> Optional[str]: + if value is None: + return None + return value.format(driver_name=driver_name, driver_version=driver_version) + # Append each DriverInfoOptions in order. for opts in appended: - d_name = opts[0] if len(opts) > 0 else None - d_version = opts[1] if len(opts) > 1 else None - d_platform = opts[2] if len(opts) > 2 else None + d_name = resolve(opts[0]) if len(opts) > 0 else None + d_version = resolve(opts[1]) if len(opts) > 1 else None + d_platform = resolve(opts[2]) if len(opts) > 2 else None client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) # New handshake with the appended metadata. name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) - assert name0 is not None - assert version0 is not None - self.assertEqual(name1, name0 + expected_name_suffix) - self.assertEqual(version1, version0 + expected_version_suffix) + self.assertEqual( + name1, + name0 + + expected_name_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + self.assertEqual( + version1, + version0 + + expected_version_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) await client.close() diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 07332a67c0..2b334904c5 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -233,7 +233,7 @@ def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") - # Prose test no. 10 + # Prose test no. 11 def test_append_metadata_rejects_delimiter(self): cases = [ ("frame|work", "2.0", "Framework Platform"), @@ -262,16 +262,26 @@ def test_append_metadata_rejects_delimiter(self): self.assertEqual(platform1, platform0) client.close() - # Prose test no. 11 + # Prose test no. 10 def test_index_correspondence(self): cases = [ + ("Gap in middle (name)", [(None, None), ("F2", None)], "||F2", "||"), ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), ("Trailing delimiter retained", [("F1", None)], "|F1", "|"), - ("Equal versions do not collapse", [("F1", None)], "|F1", "|"), - ("Equal names do not collapse", [("PyMongo", "1.0")], "|PyMongo", "|1.0"), + ( + "Equal versions do not collapse", + [("F1", "{driver_version}")], + "|F1", + "|{driver_version}", + ), + ( + "Equal names do not collapse", + [("{driver_name}", "1.0")], + "|{driver_name}", + "|1.0", + ), ("Duplicates still deduplicate", [("F1", "1.0"), ("F1", "1.0")], "|F1", "|1.0"), ("All versions absent", [("F1", None), ("F2", None)], "|F1|F2", "||"), - ("Gap in middle (name)", [(None, "1.0"), ("F2", "2.0")], "||F2", "|1.0|2.0"), ("All names absent", [(None, "1.0"), (None, "2.0")], "||", "|1.0|2.0"), ( "Non-adjacent duplicate", @@ -285,7 +295,12 @@ def test_index_correspondence(self): "|F1|F1", "|1.0|1.0", ), - ("Wrapper matching the driver's own identity", [("PyMongo", None)], "|PyMongo", "|"), + ( + "Wrapper matching the driver's own identity", + [("{driver_name}", "{driver_version}")], + "|{driver_name}", + "|{driver_version}", + ), ] for ( description, @@ -302,20 +317,40 @@ def test_index_correspondence(self): name0, version0, _, _ = self.send_ping_and_get_metadata(client, True) time.sleep(0.005) + assert name0 is not None + assert version0 is not None + driver_name = name0.split("|")[0] + driver_version = version0.split("|")[0] + + def resolve(value: Optional[str]) -> Optional[str]: + if value is None: + return None + return value.format(driver_name=driver_name, driver_version=driver_version) + # Append each DriverInfoOptions in order. for opts in appended: - d_name = opts[0] if len(opts) > 0 else None - d_version = opts[1] if len(opts) > 1 else None - d_platform = opts[2] if len(opts) > 2 else None + d_name = resolve(opts[0]) if len(opts) > 0 else None + d_version = resolve(opts[1]) if len(opts) > 1 else None + d_platform = resolve(opts[2]) if len(opts) > 2 else None client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) # New handshake with the appended metadata. name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) - assert name0 is not None - assert version0 is not None - self.assertEqual(name1, name0 + expected_name_suffix) - self.assertEqual(version1, version0 + expected_version_suffix) + self.assertEqual( + name1, + name0 + + expected_name_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + self.assertEqual( + version1, + version0 + + expected_version_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) client.close() From 62e83d40cfcef30e3599135c334675ab75234e6e Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 08:06:07 -0500 Subject: [PATCH 12/21] PYTHON-6040 Rename prose tests with their specification titles Prefix each client metadata prose test method with its prose test number and full specification title (Test 1, 2, 9, 10, 11) and order them by prose test number. --- test/asynchronous/test_client_metadata.py | 79 +++++++++++------------ test/test_client_metadata.py | 79 +++++++++++------------ 2 files changed, 76 insertions(+), 82 deletions(-) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index e7292c299a..5fafd4b630 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -114,7 +114,7 @@ async def check_metadata_added( new_metadata.pop("platform") self.assertEqual(metadata, new_metadata) - async def test_append_metadata(self): + async def test_1_test_that_the_driver_updates_metadata(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -122,7 +122,7 @@ async def test_append_metadata(self): ) await self.check_metadata_added(client, "framework", "2.0", "Framework Platform") - async def test_append_metadata_platform_none(self): + async def test_1_test_that_the_driver_updates_metadata_platform_none(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -130,7 +130,7 @@ async def test_append_metadata_platform_none(self): ) await self.check_metadata_added(client, "framework", "2.0", None) - async def test_append_metadata_version_none(self): + async def test_1_test_that_the_driver_updates_metadata_version_none(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -138,7 +138,7 @@ async def test_append_metadata_version_none(self): ) await self.check_metadata_added(client, "framework", None, "Framework Platform") - async def test_append_metadata_platform_version_none(self): + async def test_1_test_that_the_driver_updates_metadata_platform_version_none(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -146,14 +146,14 @@ async def test_append_metadata_platform_version_none(self): ) await self.check_metadata_added(client, "framework", None, None) - async def test_multiple_successive_metadata_updates(self): + async def test_2_multiple_successive_metadata_updates(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, connect=False ) client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) await self.check_metadata_added(client, "framework", "2.0", "Framework Platform") - async def test_multiple_successive_metadata_updates_platform_none(self): + async def test_2_multiple_successive_metadata_updates_platform_none(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -161,7 +161,7 @@ async def test_multiple_successive_metadata_updates_platform_none(self): client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) await self.check_metadata_added(client, "framework", "2.0", None) - async def test_multiple_successive_metadata_updates_version_none(self): + async def test_2_multiple_successive_metadata_updates_version_none(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -169,7 +169,7 @@ async def test_multiple_successive_metadata_updates_version_none(self): client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) await self.check_metadata_added(client, "framework", None, "Framework Platform") - async def test_multiple_successive_metadata_updates_platform_version_none(self): + async def test_2_multiple_successive_metadata_updates_platform_version_none(self): client = await self.async_rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -219,8 +219,7 @@ async def test_duplicate_driver_name_no_op(self): self.assertEqual(new_version, version) self.assertEqual(new_platform, platform) - # Prose test no. 9 - async def test_handshake_documents_include_backpressure(self): + async def test_9_handshake_documents_include_backpressure(self): # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of # connection establishment. client = await self.async_rs_or_single_client("mongodb://" + self.server.address_string) @@ -233,37 +232,7 @@ async def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") - # Prose test no. 11 - async def test_append_metadata_rejects_delimiter(self): - cases = [ - ("frame|work", "2.0", "Framework Platform"), - ("framework", "2|0", "Framework Platform"), - ("framework", "2.0", "Framework|Platform"), - ] - for name, version, platform in cases: - with self.subTest(name=name, version=version, platform=platform): - client = await self.async_rs_or_single_client( - "mongodb://" + self.server.address_string, - maxIdleTimeMS=1, - driver=DriverInfo("library", "1.2", "Library Platform"), - ) - # Send initial handshake. - name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( - client, True - ) - await asyncio.sleep(0.005) - # Appending metadata containing the delimiter raises. - with self.assertRaises(ValueError): - DriverInfo(name, version, platform) - # Metadata is unchanged on the next handshake. - name1, version1, platform1, _ = await self.send_ping_and_get_metadata(client, True) - self.assertEqual(name1, name0) - self.assertEqual(version1, version0) - self.assertEqual(platform1, platform0) - await client.close() - - # Prose test no. 10 - async def test_index_correspondence(self): + async def test_10_entries_in_driver_name_and_driver_version_correspond_by_index(self): cases = [ ("Gap in middle (name)", [(None, None), ("F2", None)], "||F2", "||"), ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), @@ -353,6 +322,34 @@ def resolve(value: Optional[str]) -> Optional[str]: ) await client.close() + async def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): + cases = [ + ("frame|work", "2.0", "Framework Platform"), + ("framework", "2|0", "Framework Platform"), + ("framework", "2.0", "Framework|Platform"), + ] + for name, version, platform in cases: + with self.subTest(name=name, version=version, platform=platform): + client = await self.async_rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + # Send initial handshake. + name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( + client, True + ) + await asyncio.sleep(0.005) + # Appending metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = await self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + await client.close() + if __name__ == "__main__": unittest.main() diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 2b334904c5..c3d837f5e2 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -114,7 +114,7 @@ def check_metadata_added( new_metadata.pop("platform") self.assertEqual(metadata, new_metadata) - def test_append_metadata(self): + def test_1_test_that_the_driver_updates_metadata(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -122,7 +122,7 @@ def test_append_metadata(self): ) self.check_metadata_added(client, "framework", "2.0", "Framework Platform") - def test_append_metadata_platform_none(self): + def test_1_test_that_the_driver_updates_metadata_platform_none(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -130,7 +130,7 @@ def test_append_metadata_platform_none(self): ) self.check_metadata_added(client, "framework", "2.0", None) - def test_append_metadata_version_none(self): + def test_1_test_that_the_driver_updates_metadata_version_none(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -138,7 +138,7 @@ def test_append_metadata_version_none(self): ) self.check_metadata_added(client, "framework", None, "Framework Platform") - def test_append_metadata_platform_version_none(self): + def test_1_test_that_the_driver_updates_metadata_platform_version_none(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -146,14 +146,14 @@ def test_append_metadata_platform_version_none(self): ) self.check_metadata_added(client, "framework", None, None) - def test_multiple_successive_metadata_updates(self): + def test_2_multiple_successive_metadata_updates(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, connect=False ) client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) self.check_metadata_added(client, "framework", "2.0", "Framework Platform") - def test_multiple_successive_metadata_updates_platform_none(self): + def test_2_multiple_successive_metadata_updates_platform_none(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -161,7 +161,7 @@ def test_multiple_successive_metadata_updates_platform_none(self): client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) self.check_metadata_added(client, "framework", "2.0", None) - def test_multiple_successive_metadata_updates_version_none(self): + def test_2_multiple_successive_metadata_updates_version_none(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -169,7 +169,7 @@ def test_multiple_successive_metadata_updates_version_none(self): client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) self.check_metadata_added(client, "framework", None, "Framework Platform") - def test_multiple_successive_metadata_updates_platform_version_none(self): + def test_2_multiple_successive_metadata_updates_platform_version_none(self): client = self.rs_or_single_client( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, @@ -219,8 +219,7 @@ def test_duplicate_driver_name_no_op(self): self.assertEqual(new_version, version) self.assertEqual(new_platform, platform) - # Prose test no. 9 - def test_handshake_documents_include_backpressure(self): + def test_9_handshake_documents_include_backpressure(self): # Create a `MongoClient` that is configured to record all handshake documents sent to the server as a part of # connection establishment. client = self.rs_or_single_client("mongodb://" + self.server.address_string) @@ -233,37 +232,7 @@ def test_handshake_documents_include_backpressure(self): # the document has a field `backpressure` whose value is `"2"`. self.assertEqual(self.handshake_req["backpressure"], "2") - # Prose test no. 11 - def test_append_metadata_rejects_delimiter(self): - cases = [ - ("frame|work", "2.0", "Framework Platform"), - ("framework", "2|0", "Framework Platform"), - ("framework", "2.0", "Framework|Platform"), - ] - for name, version, platform in cases: - with self.subTest(name=name, version=version, platform=platform): - client = self.rs_or_single_client( - "mongodb://" + self.server.address_string, - maxIdleTimeMS=1, - driver=DriverInfo("library", "1.2", "Library Platform"), - ) - # Send initial handshake. - name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( - client, True - ) - time.sleep(0.005) - # Appending metadata containing the delimiter raises. - with self.assertRaises(ValueError): - DriverInfo(name, version, platform) - # Metadata is unchanged on the next handshake. - name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) - self.assertEqual(name1, name0) - self.assertEqual(version1, version0) - self.assertEqual(platform1, platform0) - client.close() - - # Prose test no. 10 - def test_index_correspondence(self): + def test_10_entries_in_driver_name_and_driver_version_correspond_by_index(self): cases = [ ("Gap in middle (name)", [(None, None), ("F2", None)], "||F2", "||"), ("Gap in middle (version)", [("F1", None), ("F2", "2.0")], "|F1|F2", "||2.0"), @@ -353,6 +322,34 @@ def resolve(value: Optional[str]) -> Optional[str]: ) client.close() + def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): + cases = [ + ("frame|work", "2.0", "Framework Platform"), + ("framework", "2|0", "Framework Platform"), + ("framework", "2.0", "Framework|Platform"), + ] + for name, version, platform in cases: + with self.subTest(name=name, version=version, platform=platform): + client = self.rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + driver=DriverInfo("library", "1.2", "Library Platform"), + ) + # Send initial handshake. + name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( + client, True + ) + time.sleep(0.005) + # Appending metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + client.close() + if __name__ == "__main__": unittest.main() From 7ff17732a7f594a875d597996057eba9c8d6a7ad Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 10:53:42 -0500 Subject: [PATCH 13/21] PYTHON-6040 Extend metadata unit test coverage Exercise the DriverInfo delimiter ValueError for every field, the platform-recreation path, and add truncation/bounded-retention coverage so the new pool_options and driver_info lines are covered without mockupdb. --- test/asynchronous/test_client.py | 17 +++++++++++++++++ test/test_client.py | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 2a81e3188b..55161d5d82 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -512,6 +512,23 @@ async def test_metadata(self): for i in range(300, 600): client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + # The '|' delimiter is reserved for joining appended metadata, so it + # must be rejected in every field. + self.assertRaises(ValueError, DriverInfo, "a|b", "1.0", None) + self.assertRaises(ValueError, DriverInfo, "lib", "1|0", None) + self.assertRaises(ValueError, DriverInfo, "lib", "1.0", "Frame|Platform") + # Appending a platform after truncation has dropped it recreates the field. + client = self.simple_client(connect=False) + for i in range(300): + client.append_metadata(DriverInfo(name="", version="", platform=f"Q{i}")) + pool = client.options.pool_options + self.assertLess(len(pool._PoolOptions__appended_drivers), 300) + client.append_metadata(DriverInfo(name="Wrapper", version="1.0", platform="Recreated")) + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) + self.assertEqual( + pool.metadata["driver"]["name"].count("|"), + pool.metadata["driver"]["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index 21217c8f34..18e323c42b 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -505,6 +505,23 @@ def test_metadata(self): for i in range(300, 600): client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + # The '|' delimiter is reserved for joining appended metadata, so it + # must be rejected in every field. + self.assertRaises(ValueError, DriverInfo, "a|b", "1.0", None) + self.assertRaises(ValueError, DriverInfo, "lib", "1|0", None) + self.assertRaises(ValueError, DriverInfo, "lib", "1.0", "Frame|Platform") + # Appending a platform after truncation has dropped it recreates the field. + client = self.simple_client(connect=False) + for i in range(300): + client.append_metadata(DriverInfo(name="", version="", platform=f"Q{i}")) + pool = client.options.pool_options + self.assertLess(len(pool._PoolOptions__appended_drivers), 300) + client.append_metadata(DriverInfo(name="Wrapper", version="1.0", platform="Recreated")) + self.assertLessEqual(len(bson.encode(pool.metadata)), _MAX_METADATA_SIZE) + self.assertEqual( + pool.metadata["driver"]["name"].count("|"), + pool.metadata["driver"]["version"].count("|"), + ) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): From affb33875465dfdde9468b542a8186064084c738 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 11:22:27 -0500 Subject: [PATCH 14/21] PYTHON-6040 Trim wrapper name before dropping metadata pair When the trailing version entry is empty, shrink the oversized wrapper name instead of dropping the whole name/version pair, so a driver with a large name and no version keeps a truncated name rather than collapsing to the base entry. --- pymongo/pool_options.py | 7 +++++-- test/asynchronous/test_client.py | 16 ++++++++++++++++ test/test_client.py | 16 ++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index ce61c8e62b..afe680fbaf 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -238,8 +238,8 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: # 5. Truncate driver info, keeping name and version 1:1 index-aligned. driver = metadata.get("driver", {}) if driver: - # Trim wrapper version content first, dropping paired segments only as - # a last resort, so name and version stay 1:1 aligned. + # Trim wrapper version and name content first, dropping paired segments + # only as a last resort, so name and version stay 1:1 aligned. while True: encoded_size = len(bson.encode(metadata)) if encoded_size <= _MAX_METADATA_SIZE: @@ -252,6 +252,9 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: if len(v_parts) > 1 and v_parts[-1]: v_parts[-1] = v_parts[-1][:-overflow] driver["version"] = "|".join(v_parts) + elif len(n_parts) > 1 and n_parts[-1]: + n_parts[-1] = n_parts[-1][:-overflow] + driver["name"] = "|".join(n_parts) elif len(n_parts) > 1: n_parts.pop() v_parts.pop() diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 55161d5d82..7f7eb37287 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -486,6 +486,22 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # An oversized wrapper name with no version must retain a truncated + # name rather than collapse to the base entry. + client = self.simple_client( + driver=DriverInfo(name="x" * (_MAX_METADATA_SIZE * 2), version=None), + connect=False, + ) + truncated = client.options.pool_options.metadata["driver"] + self.assertLessEqual( + len(bson.encode(client.options.pool_options.metadata)), + _MAX_METADATA_SIZE, + ) + self.assertIn("xxxx", truncated["name"]) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) # Successive appends must stay within the limit and keep name and # version index-aligned after truncation. Once the metadata saturates, # further appends must not grow the dedup tracking list. diff --git a/test/test_client.py b/test/test_client.py index 18e323c42b..9c1bbaa5be 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -479,6 +479,22 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + # An oversized wrapper name with no version must retain a truncated + # name rather than collapse to the base entry. + client = self.simple_client( + driver=DriverInfo(name="x" * (_MAX_METADATA_SIZE * 2), version=None), + connect=False, + ) + truncated = client.options.pool_options.metadata["driver"] + self.assertLessEqual( + len(bson.encode(client.options.pool_options.metadata)), + _MAX_METADATA_SIZE, + ) + self.assertIn("xxxx", truncated["name"]) + self.assertEqual( + truncated["name"].count("|"), + truncated["version"].count("|"), + ) # Successive appends must stay within the limit and keep name and # version index-aligned after truncation. Once the metadata saturates, # further appends must not grow the dedup tracking list. From 6ef9ab064cf8196203e46d85a92ee96e04cf3478 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 11:49:47 -0500 Subject: [PATCH 15/21] PYTHON-6040 Truncate metadata by UTF-8 bytes The overflow is a BSON byte count, so trim wrapper version, name, and platform by UTF-8 bytes and decode a valid prefix instead of slicing Unicode code points. --- pymongo/pool_options.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index afe680fbaf..966fa8de45 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -201,6 +201,16 @@ def _metadata_env() -> dict[str, Any]: _MAX_METADATA_SIZE = 512 +def _truncate_utf8(content: str, overflow: int) -> str: + """Trim `overflow` UTF-8 bytes from the end of content, keeping a valid prefix.""" + if overflow <= 0: + return content + data = content.encode("utf-8") + if len(data) <= overflow: + return "" + return data[: len(data) - overflow].decode("utf-8", errors="ignore") + + # See: https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.md#limitations def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: """Perform metadata truncation.""" @@ -227,7 +237,7 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: overflow = encoded_size - _MAX_METADATA_SIZE plat = metadata.get("platform", "") if plat: - plat = plat[:-overflow] + plat = _truncate_utf8(plat, overflow) if plat: metadata["platform"] = plat else: @@ -250,10 +260,10 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: v_parts = driver.get("version", "").split("|") if len(v_parts) > 1 and v_parts[-1]: - v_parts[-1] = v_parts[-1][:-overflow] + v_parts[-1] = _truncate_utf8(v_parts[-1], overflow) driver["version"] = "|".join(v_parts) elif len(n_parts) > 1 and n_parts[-1]: - n_parts[-1] = n_parts[-1][:-overflow] + n_parts[-1] = _truncate_utf8(n_parts[-1], overflow) driver["name"] = "|".join(n_parts) elif len(n_parts) > 1: n_parts.pop() From e3ead2a08fcb14d9b92a71d9fa6f2a62223bc0d2 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 12:44:01 -0500 Subject: [PATCH 16/21] PYTHON-6040 Dedup metadata across None and empty strings Normalize DriverInfo fields before the dedup comparison so None and '' are treated as the same unset value, matching the spec and avoiding duplicate name/version segments. --- pymongo/pool_options.py | 10 ++++++++++ test/asynchronous/test_client.py | 10 ++++++++++ test/test_client.py | 10 ++++++++++ 3 files changed, 30 insertions(+) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 966fa8de45..a822952e6d 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -211,6 +211,15 @@ def _truncate_utf8(content: str, overflow: int) -> str: return data[: len(data) - overflow].decode("utf-8", errors="ignore") +def _normalize_driver(driver: DriverInfo) -> DriverInfo: + """Treat None and "" as equivalent unset fields for deduplication.""" + return driver._replace( + name=driver.name or "", + version=driver.version or "", + platform=driver.platform or "", + ) + + # See: https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.md#limitations def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: """Perform metadata truncation.""" @@ -403,6 +412,7 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata.""" with self.__metadata_lock: + driver = _normalize_driver(driver) if driver in self.__appended_drivers: return diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 7f7eb37287..d4f16487e2 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -545,6 +545,16 @@ async def test_metadata(self): pool.metadata["driver"]["name"].count("|"), pool.metadata["driver"]["version"].count("|"), ) + # Empty strings are treated as unset, so a duplicate differing only in + # None vs "" is a no-op. + client = self.simple_client(connect=False) + client.append_metadata(DriverInfo("library", None, "Library Platform")) + names = client.options.pool_options.metadata["driver"]["name"] + vers = client.options.pool_options.metadata["driver"]["version"] + client.append_metadata(DriverInfo("library", "", "Library Platform")) + metadata = client.options.pool_options.metadata + self.assertEqual(metadata["driver"]["name"], names) + self.assertEqual(metadata["driver"]["version"], vers) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): diff --git a/test/test_client.py b/test/test_client.py index 9c1bbaa5be..499da31b40 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -538,6 +538,16 @@ def test_metadata(self): pool.metadata["driver"]["name"].count("|"), pool.metadata["driver"]["version"].count("|"), ) + # Empty strings are treated as unset, so a duplicate differing only in + # None vs "" is a no-op. + client = self.simple_client(connect=False) + client.append_metadata(DriverInfo("library", None, "Library Platform")) + names = client.options.pool_options.metadata["driver"]["name"] + vers = client.options.pool_options.metadata["driver"]["version"] + client.append_metadata(DriverInfo("library", "", "Library Platform")) + metadata = client.options.pool_options.metadata + self.assertEqual(metadata["driver"]["name"], names) + self.assertEqual(metadata["driver"]["version"], vers) @mock.patch.dict("os.environ", {ENV_VAR_K8S: "1"}) def test_container_metadata(self): From 917bc0abb12d947156dbc98fb11bab0eee42c01e Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 20:18:21 -0500 Subject: [PATCH 17/21] Update pymongo/pool_options.py Co-authored-by: Noah Stapp --- pymongo/pool_options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index a822952e6d..417b462d0e 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -264,7 +264,7 @@ def _truncate_metadata(metadata: MutableMapping[str, Any]) -> None: if encoded_size <= _MAX_METADATA_SIZE: break overflow = encoded_size - _MAX_METADATA_SIZE - previous = (driver.get("name"), driver.get("version")) + previous = (driver.get("name", ""), driver.get("version", "")) n_parts = driver.get("name", "").split("|") v_parts = driver.get("version", "").split("|") From 66ce4191ec209819030aba5503b29b85e4cd6378 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 20:18:35 -0500 Subject: [PATCH 18/21] Update test/asynchronous/test_client_metadata.py Co-authored-by: Noah Stapp --- test/asynchronous/test_client_metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 5fafd4b630..2e9d83221b 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -296,7 +296,7 @@ def resolve(value: Optional[str]) -> Optional[str]: return None return value.format(driver_name=driver_name, driver_version=driver_version) - # Append each DriverInfoOptions in order. + # Append each DriverInfo in order. for opts in appended: d_name = resolve(opts[0]) if len(opts) > 0 else None d_version = resolve(opts[1]) if len(opts) > 1 else None From e8c253c9dba4eb38b1803c67d7c48191e34f0f80 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 20:41:21 -0500 Subject: [PATCH 19/21] PYTHON-6040 Address PR review feedback Scope the metadata lock to the synchronous client and track appended name/version pairs by both fields. Split test_metadata into focused tests, close test clients in a finally block, and document the whole-DriverInfo dedup and reserved delimiter in the changelog. --- doc/changelog.rst | 7 ++ pymongo/pool_options.py | 65 +++++++----- test/asynchronous/test_client.py | 72 +++++++------- test/asynchronous/test_client_metadata.py | 115 ++++++++++++---------- test/test_client.py | 72 +++++++------- test/test_client_metadata.py | 113 +++++++++++---------- test/utils_shared.py | 13 +++ 7 files changed, 254 insertions(+), 203 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 41e6dee233..60ff019b4b 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -10,8 +10,15 @@ Bug fixes - Fixed a bug where the synchronous client could permanently deadlock under gevent when a greenlet was killed while checking a connection back into the pool (`PYTHON-6074`_). +- ``MongoClient.append_metadata()`` and ``AsyncMongoClient.append_metadata()`` + now detect duplicates by comparing the whole + :class:`~pymongo.driver_info.DriverInfo` instead of only its name + (`PYTHON-6040`_). +- :class:`~pymongo.driver_info.DriverInfo` now raises :class:`ValueError` when + any field contains the reserved ``|`` delimiter (`PYTHON-6040`_). .. _PYTHON-6074: https://jira.mongodb.org/browse/PYTHON-6074 +.. _PYTHON-6040: https://jira.mongodb.org/browse/PYTHON-6040 Changes in Version 4.18.1 (2026/09/10) -------------------------------------- diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 417b462d0e..1aefcbc862 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -23,6 +23,7 @@ import os import platform import sys +import threading from collections.abc import MutableMapping from pathlib import Path from typing import TYPE_CHECKING, Any, Optional @@ -369,7 +370,8 @@ def __init__( self.__credentials = credentials self.__metadata = copy.deepcopy(_METADATA) self.__appended_drivers: list[DriverInfo] = [] - self.__metadata_lock = _create_lock() + # Only the synchronous client can append metadata from multiple threads. + self.__metadata_lock: Optional[threading.Lock] = _create_lock() if is_sync else None if appname: self.__metadata["application"] = {"name": appname} @@ -411,35 +413,44 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata.""" - with self.__metadata_lock: - driver = _normalize_driver(driver) - if driver in self.__appended_drivers: - return - - name_delims = self.__metadata["driver"]["name"].count("|") - metadata = copy.deepcopy(self.__metadata) - - metadata["driver"]["name"] = "{}|{}".format( - metadata["driver"]["name"], driver.name or "" - ) - metadata["driver"]["version"] = "{}|{}".format( - metadata["driver"]["version"], driver.version or "" - ) - if driver.platform: - if "platform" in metadata: - metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) - else: - metadata["platform"] = driver.platform + lock = self.__metadata_lock + if lock is None: + self._apply_metadata(driver) + else: + with lock: + self._apply_metadata(driver) + + def _apply_metadata(self, driver: DriverInfo) -> None: + driver = _normalize_driver(driver) + if driver in self.__appended_drivers: + return + + name_delims = self.__metadata["driver"]["name"].count("|") + version_delims = self.__metadata["driver"]["version"].count("|") + metadata = copy.deepcopy(self.__metadata) + + metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name) + metadata["driver"]["version"] = "{}|{}".format( + metadata["driver"]["version"], driver.version + ) + if driver.platform: + if "platform" in metadata: + metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + else: + metadata["platform"] = driver.platform - _truncate_metadata(metadata) + _truncate_metadata(metadata) - self.__metadata = metadata + self.__metadata = metadata - # Only track drivers whose appended name/version pair survived - # truncation (i.e. the name gained a segment), so __appended_drivers - # stays bounded and the dedup membership check stays fast. - if metadata["driver"]["name"].count("|") > name_delims: - self.__appended_drivers.append(driver) + # Only track drivers whose appended name/version pair survived + # truncation (i.e. both gained a segment), so __appended_drivers + # stays bounded and the dedup membership check stays fast. + if ( + metadata["driver"]["name"].count("|") > name_delims + and metadata["driver"]["version"].count("|") > version_delims + ): + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index d4f16487e2..b3b5a7e648 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -124,6 +124,7 @@ NTHREADS, CMAPListener, FunctionCallRecorder, + _driver_version, delay, gevent_monkey_patched, is_greenthread_patched, @@ -134,19 +135,6 @@ _IS_SYNC = False -def _driver_version(base_version: str, name: str, last_version: str | None = None) -> str: - """Build a metadata driver version aligned 1:1 with ``name`` segments. - - The ``|c`` and ``|async`` name segments always have an empty version entry, - so the version string has one delimiter per name delimiter. ``last_version`` - is used when the final segment carries a wrapped driver's version. - """ - segments = [""] * name.count("|") - if last_version is not None: - segments[-1] = last_version - return "|".join([base_version, *segments]) - - class AsyncClientUnitTest(AsyncUnitTest): """AsyncMongoClient tests that don't require a server.""" @@ -393,6 +381,22 @@ async def test_read_preference(self): ) self.assertEqual(c.read_preference, ReadPreference.NEAREST) + def _metadata_with_appended_driver( + self, name: str, version: str, platform: str | None = None + ) -> dict[str, Any]: + metadata = copy.deepcopy(_METADATA) + if has_c(): + metadata["driver"]["name"] = "PyMongo|c|async|" + name + else: + metadata["driver"]["name"] = "PyMongo|async|" + name + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version=version + ) + metadata["application"] = {"name": "foobar"} + if platform is not None: + metadata["platform"] = "{}|{}".format(_METADATA["platform"], platform) + return metadata + async def test_metadata(self): metadata = copy.deepcopy(_METADATA) if has_c(): @@ -413,6 +417,8 @@ async def test_metadata(self): self.simple_client(appname="x" * 128) with self.assertRaises(ValueError): self.simple_client(appname="x" * 129) + + async def test_metadata_bad_driver_options(self): # Bad "driver" options. self.assertRaises(TypeError, DriverInfo, "Foo", 1, "a") self.assertRaises(TypeError, DriverInfo, version="1", platform="a") @@ -423,14 +429,9 @@ async def test_metadata(self): self.simple_client(driver="abc") with self.assertRaises(TypeError): self.simple_client(driver=("Foo", "1", "a")) - # Test appending to driver info. - if has_c(): - metadata["driver"]["name"] = "PyMongo|c|async|FooDriver" - else: - metadata["driver"]["name"] = "PyMongo|async|FooDriver" - metadata["driver"]["version"] = _driver_version( - _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" - ) + + async def test_metadata_appends_driver_info(self): + metadata = self._metadata_with_appended_driver("FooDriver", "1.2.3") client = self.simple_client( "foo", 27017, @@ -438,16 +439,9 @@ async def test_metadata(self): driver=DriverInfo("FooDriver", "1.2.3", None), connect=False, ) - options = client.options - self.assertEqual(options.pool_options.metadata, metadata) - if has_c(): - metadata["driver"]["name"] = "PyMongo|c|async|FooDriver" - else: - metadata["driver"]["name"] = "PyMongo|async|FooDriver" - metadata["driver"]["version"] = _driver_version( - _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" - ) - metadata["platform"] = "{}|FooPlatform".format(_METADATA["platform"]) + self.assertEqual(client.options.pool_options.metadata, metadata) + + metadata = self._metadata_with_appended_driver("FooDriver", "1.2.3", "FooPlatform") client = self.simple_client( "foo", 27017, @@ -455,9 +449,11 @@ async def test_metadata(self): driver=DriverInfo("FooDriver", "1.2.3", "FooPlatform"), connect=False, ) - options = client.options - self.assertEqual(options.pool_options.metadata, metadata) - # Test truncating driver info metadata. + self.assertEqual(client.options.pool_options.metadata, metadata) + + async def test_metadata_truncates_driver_info(self): + # Truncated driver info must stay within the limit and keep name and + # version index-aligned. client = self.simple_client( driver=DriverInfo(name="s" * _MAX_METADATA_SIZE), connect=False, @@ -502,6 +498,8 @@ async def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + + async def test_metadata_append_is_bounded(self): # Successive appends must stay within the limit and keep name and # version index-aligned after truncation. Once the metadata saturates, # further appends must not grow the dedup tracking list. @@ -528,11 +526,15 @@ async def test_metadata(self): for i in range(300, 600): client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + + async def test_metadata_rejects_delimiter(self): # The '|' delimiter is reserved for joining appended metadata, so it # must be rejected in every field. self.assertRaises(ValueError, DriverInfo, "a|b", "1.0", None) self.assertRaises(ValueError, DriverInfo, "lib", "1|0", None) self.assertRaises(ValueError, DriverInfo, "lib", "1.0", "Frame|Platform") + + async def test_metadata_recreates_platform_after_truncation(self): # Appending a platform after truncation has dropped it recreates the field. client = self.simple_client(connect=False) for i in range(300): @@ -545,6 +547,8 @@ async def test_metadata(self): pool.metadata["driver"]["name"].count("|"), pool.metadata["driver"]["version"].count("|"), ) + + async def test_metadata_deduplicates_none_and_empty(self): # Empty strings are treated as unset, so a duplicate differing only in # None vs "" is a no-op. client = self.simple_client(connect=False) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 2e9d83221b..1fad85e3d2 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -18,7 +18,7 @@ import pathlib import time import unittest -from typing import Any, Optional +from typing import Any, Optional, cast import pytest @@ -282,45 +282,48 @@ async def test_10_entries_in_driver_name_and_driver_version_correspond_by_index( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, ) - # Capture the driver's own name and version from the first handshake. - name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True) - await asyncio.sleep(0.005) - - assert name0 is not None - assert version0 is not None - driver_name = name0.split("|")[0] - driver_version = version0.split("|")[0] - - def resolve(value: Optional[str]) -> Optional[str]: - if value is None: - return None - return value.format(driver_name=driver_name, driver_version=driver_version) - - # Append each DriverInfo in order. - for opts in appended: - d_name = resolve(opts[0]) if len(opts) > 0 else None - d_version = resolve(opts[1]) if len(opts) > 1 else None - d_platform = resolve(opts[2]) if len(opts) > 2 else None - client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) - - # New handshake with the appended metadata. - name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) - - self.assertEqual( - name1, - name0 - + expected_name_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - self.assertEqual( - version1, - version0 - + expected_version_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - await client.close() + try: + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True) + await asyncio.sleep(0.005) + + self.assertIsNotNone(name0) + self.assertIsNotNone(version0) + version0 = cast(str, version0) + driver_name = name0.split("|")[0] + driver_version = version0.split("|")[0] + + def resolve(value: Optional[str]) -> Optional[str]: + if value is None: + return None + return value.format(driver_name=driver_name, driver_version=driver_version) + + # Append each DriverInfo in order. + for opts in appended: + d_name = resolve(opts[0]) if len(opts) > 0 else None + d_version = resolve(opts[1]) if len(opts) > 1 else None + d_platform = resolve(opts[2]) if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) + + self.assertEqual( + name1, + name0 + + expected_name_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + self.assertEqual( + version1, + version0 + + expected_version_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + finally: + await client.close() async def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): cases = [ @@ -335,20 +338,24 @@ async def test_11_appending_metadata_containing_the_delimiter_raises_an_error(se maxIdleTimeMS=1, driver=DriverInfo("library", "1.2", "Library Platform"), ) - # Send initial handshake. - name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( - client, True - ) - await asyncio.sleep(0.005) - # Appending metadata containing the delimiter raises. - with self.assertRaises(ValueError): - DriverInfo(name, version, platform) - # Metadata is unchanged on the next handshake. - name1, version1, platform1, _ = await self.send_ping_and_get_metadata(client, True) - self.assertEqual(name1, name0) - self.assertEqual(version1, version0) - self.assertEqual(platform1, platform0) - await client.close() + try: + # Send initial handshake. + name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( + client, True + ) + await asyncio.sleep(0.005) + # Constructing metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = await self.send_ping_and_get_metadata( + client, True + ) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + finally: + await client.close() if __name__ == "__main__": diff --git a/test/test_client.py b/test/test_client.py index 499da31b40..7a6833753c 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -123,6 +123,7 @@ NTHREADS, CMAPListener, FunctionCallRecorder, + _driver_version, delay, gevent_monkey_patched, is_greenthread_patched, @@ -133,19 +134,6 @@ _IS_SYNC = True -def _driver_version(base_version: str, name: str, last_version: str | None = None) -> str: - """Build a metadata driver version aligned 1:1 with ``name`` segments. - - The ``|c`` and ``|async`` name segments always have an empty version entry, - so the version string has one delimiter per name delimiter. ``last_version`` - is used when the final segment carries a wrapped driver's version. - """ - segments = [""] * name.count("|") - if last_version is not None: - segments[-1] = last_version - return "|".join([base_version, *segments]) - - class ClientUnitTest(UnitTest): """MongoClient tests that don't require a server.""" @@ -386,6 +374,22 @@ def test_read_preference(self): ) self.assertEqual(c.read_preference, ReadPreference.NEAREST) + def _metadata_with_appended_driver( + self, name: str, version: str, platform: str | None = None + ) -> dict[str, Any]: + metadata = copy.deepcopy(_METADATA) + if has_c(): + metadata["driver"]["name"] = "PyMongo|c|" + name + else: + metadata["driver"]["name"] = "PyMongo|" + name + metadata["driver"]["version"] = _driver_version( + _METADATA["driver"]["version"], metadata["driver"]["name"], last_version=version + ) + metadata["application"] = {"name": "foobar"} + if platform is not None: + metadata["platform"] = "{}|{}".format(_METADATA["platform"], platform) + return metadata + def test_metadata(self): metadata = copy.deepcopy(_METADATA) if has_c(): @@ -406,6 +410,8 @@ def test_metadata(self): self.simple_client(appname="x" * 128) with self.assertRaises(ValueError): self.simple_client(appname="x" * 129) + + def test_metadata_bad_driver_options(self): # Bad "driver" options. self.assertRaises(TypeError, DriverInfo, "Foo", 1, "a") self.assertRaises(TypeError, DriverInfo, version="1", platform="a") @@ -416,14 +422,9 @@ def test_metadata(self): self.simple_client(driver="abc") with self.assertRaises(TypeError): self.simple_client(driver=("Foo", "1", "a")) - # Test appending to driver info. - if has_c(): - metadata["driver"]["name"] = "PyMongo|c|FooDriver" - else: - metadata["driver"]["name"] = "PyMongo|FooDriver" - metadata["driver"]["version"] = _driver_version( - _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" - ) + + def test_metadata_appends_driver_info(self): + metadata = self._metadata_with_appended_driver("FooDriver", "1.2.3") client = self.simple_client( "foo", 27017, @@ -431,16 +432,9 @@ def test_metadata(self): driver=DriverInfo("FooDriver", "1.2.3", None), connect=False, ) - options = client.options - self.assertEqual(options.pool_options.metadata, metadata) - if has_c(): - metadata["driver"]["name"] = "PyMongo|c|FooDriver" - else: - metadata["driver"]["name"] = "PyMongo|FooDriver" - metadata["driver"]["version"] = _driver_version( - _METADATA["driver"]["version"], metadata["driver"]["name"], last_version="1.2.3" - ) - metadata["platform"] = "{}|FooPlatform".format(_METADATA["platform"]) + self.assertEqual(client.options.pool_options.metadata, metadata) + + metadata = self._metadata_with_appended_driver("FooDriver", "1.2.3", "FooPlatform") client = self.simple_client( "foo", 27017, @@ -448,9 +442,11 @@ def test_metadata(self): driver=DriverInfo("FooDriver", "1.2.3", "FooPlatform"), connect=False, ) - options = client.options - self.assertEqual(options.pool_options.metadata, metadata) - # Test truncating driver info metadata. + self.assertEqual(client.options.pool_options.metadata, metadata) + + def test_metadata_truncates_driver_info(self): + # Truncated driver info must stay within the limit and keep name and + # version index-aligned. client = self.simple_client( driver=DriverInfo(name="s" * _MAX_METADATA_SIZE), connect=False, @@ -495,6 +491,8 @@ def test_metadata(self): truncated["name"].count("|"), truncated["version"].count("|"), ) + + def test_metadata_append_is_bounded(self): # Successive appends must stay within the limit and keep name and # version index-aligned after truncation. Once the metadata saturates, # further appends must not grow the dedup tracking list. @@ -521,11 +519,15 @@ def test_metadata(self): for i in range(300, 600): client.append_metadata(DriverInfo(name="", version="", platform=f"P{i}")) self.assertEqual(len(pool._PoolOptions__appended_drivers), count) + + def test_metadata_rejects_delimiter(self): # The '|' delimiter is reserved for joining appended metadata, so it # must be rejected in every field. self.assertRaises(ValueError, DriverInfo, "a|b", "1.0", None) self.assertRaises(ValueError, DriverInfo, "lib", "1|0", None) self.assertRaises(ValueError, DriverInfo, "lib", "1.0", "Frame|Platform") + + def test_metadata_recreates_platform_after_truncation(self): # Appending a platform after truncation has dropped it recreates the field. client = self.simple_client(connect=False) for i in range(300): @@ -538,6 +540,8 @@ def test_metadata(self): pool.metadata["driver"]["name"].count("|"), pool.metadata["driver"]["version"].count("|"), ) + + def test_metadata_deduplicates_none_and_empty(self): # Empty strings are treated as unset, so a duplicate differing only in # None vs "" is a no-op. client = self.simple_client(connect=False) diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index c3d837f5e2..d1574b8e85 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -18,7 +18,7 @@ import pathlib import time import unittest -from typing import Any, Optional +from typing import Any, Optional, cast import pytest @@ -282,45 +282,48 @@ def test_10_entries_in_driver_name_and_driver_version_correspond_by_index(self): "mongodb://" + self.server.address_string, maxIdleTimeMS=1, ) - # Capture the driver's own name and version from the first handshake. - name0, version0, _, _ = self.send_ping_and_get_metadata(client, True) - time.sleep(0.005) - - assert name0 is not None - assert version0 is not None - driver_name = name0.split("|")[0] - driver_version = version0.split("|")[0] - - def resolve(value: Optional[str]) -> Optional[str]: - if value is None: - return None - return value.format(driver_name=driver_name, driver_version=driver_version) - - # Append each DriverInfoOptions in order. - for opts in appended: - d_name = resolve(opts[0]) if len(opts) > 0 else None - d_version = resolve(opts[1]) if len(opts) > 1 else None - d_platform = resolve(opts[2]) if len(opts) > 2 else None - client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) - - # New handshake with the appended metadata. - name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) - - self.assertEqual( - name1, - name0 - + expected_name_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - self.assertEqual( - version1, - version0 - + expected_version_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - client.close() + try: + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = self.send_ping_and_get_metadata(client, True) + time.sleep(0.005) + + self.assertIsNotNone(name0) + self.assertIsNotNone(version0) + version0 = cast(str, version0) + driver_name = name0.split("|")[0] + driver_version = version0.split("|")[0] + + def resolve(value: Optional[str]) -> Optional[str]: + if value is None: + return None + return value.format(driver_name=driver_name, driver_version=driver_version) + + # Append each DriverInfo in order. + for opts in appended: + d_name = resolve(opts[0]) if len(opts) > 0 else None + d_version = resolve(opts[1]) if len(opts) > 1 else None + d_platform = resolve(opts[2]) if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) + + self.assertEqual( + name1, + name0 + + expected_name_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + self.assertEqual( + version1, + version0 + + expected_version_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + finally: + client.close() def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): cases = [ @@ -335,20 +338,22 @@ def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): maxIdleTimeMS=1, driver=DriverInfo("library", "1.2", "Library Platform"), ) - # Send initial handshake. - name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( - client, True - ) - time.sleep(0.005) - # Appending metadata containing the delimiter raises. - with self.assertRaises(ValueError): - DriverInfo(name, version, platform) - # Metadata is unchanged on the next handshake. - name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) - self.assertEqual(name1, name0) - self.assertEqual(version1, version0) - self.assertEqual(platform1, platform0) - client.close() + try: + # Send initial handshake. + name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( + client, True + ) + time.sleep(0.005) + # Constructing metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) + finally: + client.close() if __name__ == "__main__": diff --git a/test/utils_shared.py b/test/utils_shared.py index 6ae9405207..341e9c90dd 100644 --- a/test/utils_shared.py +++ b/test/utils_shared.py @@ -773,3 +773,16 @@ def pack_msg_header(length: int, request_id: int, response_to: int, op_code: int production header-packing never does. """ return struct.pack(" str: + """Build a metadata driver version aligned 1:1 with ``name`` segments. + + The ``|c`` and ``|async`` name segments always have an empty version entry, + so the version string has one delimiter per name delimiter. ``last_version`` + is used when the final segment carries a wrapped driver's version. + """ + segments = [""] * name.count("|") + if last_version is not None: + segments[-1] = last_version + return "|".join([base_version, *segments]) From dbc559dd3ea6155a561285efc4261e1fb1ee9bbe Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 20:55:48 -0500 Subject: [PATCH 20/21] PYTHON-6040 Use a null context for the async metadata lock Store a nullcontext for the asynchronous client so _update_metadata uses one with block for both clients. --- pymongo/pool_options.py | 75 +++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 1aefcbc862..b9a007c738 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -23,8 +23,8 @@ import os import platform import sys -import threading from collections.abc import MutableMapping +from contextlib import AbstractContextManager, nullcontext from pathlib import Path from typing import TYPE_CHECKING, Any, Optional @@ -371,7 +371,9 @@ def __init__( self.__metadata = copy.deepcopy(_METADATA) self.__appended_drivers: list[DriverInfo] = [] # Only the synchronous client can append metadata from multiple threads. - self.__metadata_lock: Optional[threading.Lock] = _create_lock() if is_sync else None + self.__metadata_lock: AbstractContextManager[bool | None] = ( + _create_lock() if is_sync else nullcontext() + ) if appname: self.__metadata["application"] = {"name": appname} @@ -413,44 +415,37 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata.""" - lock = self.__metadata_lock - if lock is None: - self._apply_metadata(driver) - else: - with lock: - self._apply_metadata(driver) - - def _apply_metadata(self, driver: DriverInfo) -> None: - driver = _normalize_driver(driver) - if driver in self.__appended_drivers: - return - - name_delims = self.__metadata["driver"]["name"].count("|") - version_delims = self.__metadata["driver"]["version"].count("|") - metadata = copy.deepcopy(self.__metadata) - - metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name) - metadata["driver"]["version"] = "{}|{}".format( - metadata["driver"]["version"], driver.version - ) - if driver.platform: - if "platform" in metadata: - metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) - else: - metadata["platform"] = driver.platform - - _truncate_metadata(metadata) - - self.__metadata = metadata - - # Only track drivers whose appended name/version pair survived - # truncation (i.e. both gained a segment), so __appended_drivers - # stays bounded and the dedup membership check stays fast. - if ( - metadata["driver"]["name"].count("|") > name_delims - and metadata["driver"]["version"].count("|") > version_delims - ): - self.__appended_drivers.append(driver) + with self.__metadata_lock: + driver = _normalize_driver(driver) + if driver in self.__appended_drivers: + return + + name_delims = self.__metadata["driver"]["name"].count("|") + version_delims = self.__metadata["driver"]["version"].count("|") + metadata = copy.deepcopy(self.__metadata) + + metadata["driver"]["name"] = "{}|{}".format(metadata["driver"]["name"], driver.name) + metadata["driver"]["version"] = "{}|{}".format( + metadata["driver"]["version"], driver.version + ) + if driver.platform: + if "platform" in metadata: + metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + else: + metadata["platform"] = driver.platform + + _truncate_metadata(metadata) + + self.__metadata = metadata + + # Only track drivers whose appended name/version pair survived + # truncation (i.e. both gained a segment), so __appended_drivers + # stays bounded and the dedup membership check stays fast. + if ( + metadata["driver"]["name"].count("|") > name_delims + and metadata["driver"]["version"].count("|") > version_delims + ): + self.__appended_drivers.append(driver) @property def _credentials(self) -> Optional[MongoCredential]: From 7ecbb94049038b6b335922a4b544693902e45c88 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 15 Sep 2026 21:04:44 -0500 Subject: [PATCH 21/21] PYTHON-6040 Close test clients with unittest cleanup Register client.close via addAsyncCleanup instead of a try/finally so clients are still closed when a subtest fails. --- test/asynchronous/test_client_metadata.py | 114 ++++++++++------------ test/test_client_metadata.py | 112 ++++++++++----------- 2 files changed, 108 insertions(+), 118 deletions(-) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 1fad85e3d2..56c72d52d6 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -282,48 +282,46 @@ async def test_10_entries_in_driver_name_and_driver_version_correspond_by_index( "mongodb://" + self.server.address_string, maxIdleTimeMS=1, ) - try: - # Capture the driver's own name and version from the first handshake. - name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True) - await asyncio.sleep(0.005) - - self.assertIsNotNone(name0) - self.assertIsNotNone(version0) - version0 = cast(str, version0) - driver_name = name0.split("|")[0] - driver_version = version0.split("|")[0] - - def resolve(value: Optional[str]) -> Optional[str]: - if value is None: - return None - return value.format(driver_name=driver_name, driver_version=driver_version) - - # Append each DriverInfo in order. - for opts in appended: - d_name = resolve(opts[0]) if len(opts) > 0 else None - d_version = resolve(opts[1]) if len(opts) > 1 else None - d_platform = resolve(opts[2]) if len(opts) > 2 else None - client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) - - # New handshake with the appended metadata. - name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) - - self.assertEqual( - name1, - name0 - + expected_name_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - self.assertEqual( - version1, - version0 - + expected_version_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - finally: - await client.close() + self.addAsyncCleanup(client.close) + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = await self.send_ping_and_get_metadata(client, True) + await asyncio.sleep(0.005) + + self.assertIsNotNone(name0) + self.assertIsNotNone(version0) + version0 = cast(str, version0) + driver_name = name0.split("|")[0] + driver_version = version0.split("|")[0] + + def resolve(value: Optional[str]) -> Optional[str]: + if value is None: + return None + return value.format(driver_name=driver_name, driver_version=driver_version) + + # Append each DriverInfo in order. + for opts in appended: + d_name = resolve(opts[0]) if len(opts) > 0 else None + d_version = resolve(opts[1]) if len(opts) > 1 else None + d_platform = resolve(opts[2]) if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = await self.send_ping_and_get_metadata(client, True) + + self.assertEqual( + name1, + name0 + + expected_name_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + self.assertEqual( + version1, + version0 + + expected_version_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) async def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): cases = [ @@ -338,24 +336,20 @@ async def test_11_appending_metadata_containing_the_delimiter_raises_an_error(se maxIdleTimeMS=1, driver=DriverInfo("library", "1.2", "Library Platform"), ) - try: - # Send initial handshake. - name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( - client, True - ) - await asyncio.sleep(0.005) - # Constructing metadata containing the delimiter raises. - with self.assertRaises(ValueError): - DriverInfo(name, version, platform) - # Metadata is unchanged on the next handshake. - name1, version1, platform1, _ = await self.send_ping_and_get_metadata( - client, True - ) - self.assertEqual(name1, name0) - self.assertEqual(version1, version0) - self.assertEqual(platform1, platform0) - finally: - await client.close() + self.addAsyncCleanup(client.close) + # Send initial handshake. + name0, version0, platform0, _metadata = await self.send_ping_and_get_metadata( + client, True + ) + await asyncio.sleep(0.005) + # Constructing metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = await self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) if __name__ == "__main__": diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index d1574b8e85..4ea3eb00ba 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -282,48 +282,46 @@ def test_10_entries_in_driver_name_and_driver_version_correspond_by_index(self): "mongodb://" + self.server.address_string, maxIdleTimeMS=1, ) - try: - # Capture the driver's own name and version from the first handshake. - name0, version0, _, _ = self.send_ping_and_get_metadata(client, True) - time.sleep(0.005) - - self.assertIsNotNone(name0) - self.assertIsNotNone(version0) - version0 = cast(str, version0) - driver_name = name0.split("|")[0] - driver_version = version0.split("|")[0] - - def resolve(value: Optional[str]) -> Optional[str]: - if value is None: - return None - return value.format(driver_name=driver_name, driver_version=driver_version) - - # Append each DriverInfo in order. - for opts in appended: - d_name = resolve(opts[0]) if len(opts) > 0 else None - d_version = resolve(opts[1]) if len(opts) > 1 else None - d_platform = resolve(opts[2]) if len(opts) > 2 else None - client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) - - # New handshake with the appended metadata. - name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) - - self.assertEqual( - name1, - name0 - + expected_name_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - self.assertEqual( - version1, - version0 - + expected_version_suffix.format( - driver_name=driver_name, driver_version=driver_version - ), - ) - finally: - client.close() + self.addCleanup(client.close) + # Capture the driver's own name and version from the first handshake. + name0, version0, _, _ = self.send_ping_and_get_metadata(client, True) + time.sleep(0.005) + + self.assertIsNotNone(name0) + self.assertIsNotNone(version0) + version0 = cast(str, version0) + driver_name = name0.split("|")[0] + driver_version = version0.split("|")[0] + + def resolve(value: Optional[str]) -> Optional[str]: + if value is None: + return None + return value.format(driver_name=driver_name, driver_version=driver_version) + + # Append each DriverInfo in order. + for opts in appended: + d_name = resolve(opts[0]) if len(opts) > 0 else None + d_version = resolve(opts[1]) if len(opts) > 1 else None + d_platform = resolve(opts[2]) if len(opts) > 2 else None + client.append_metadata(DriverInfo(d_name or "", d_version, d_platform)) + + # New handshake with the appended metadata. + name1, version1, _, _ = self.send_ping_and_get_metadata(client, True) + + self.assertEqual( + name1, + name0 + + expected_name_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) + self.assertEqual( + version1, + version0 + + expected_version_suffix.format( + driver_name=driver_name, driver_version=driver_version + ), + ) def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): cases = [ @@ -338,22 +336,20 @@ def test_11_appending_metadata_containing_the_delimiter_raises_an_error(self): maxIdleTimeMS=1, driver=DriverInfo("library", "1.2", "Library Platform"), ) - try: - # Send initial handshake. - name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( - client, True - ) - time.sleep(0.005) - # Constructing metadata containing the delimiter raises. - with self.assertRaises(ValueError): - DriverInfo(name, version, platform) - # Metadata is unchanged on the next handshake. - name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) - self.assertEqual(name1, name0) - self.assertEqual(version1, version0) - self.assertEqual(platform1, platform0) - finally: - client.close() + self.addCleanup(client.close) + # Send initial handshake. + name0, version0, platform0, _metadata = self.send_ping_and_get_metadata( + client, True + ) + time.sleep(0.005) + # Constructing metadata containing the delimiter raises. + with self.assertRaises(ValueError): + DriverInfo(name, version, platform) + # Metadata is unchanged on the next handshake. + name1, version1, platform1, _ = self.send_ping_and_get_metadata(client, True) + self.assertEqual(name1, name0) + self.assertEqual(version1, version0) + self.assertEqual(platform1, platform0) if __name__ == "__main__":