From ba6d70078c3c05a95d6cc8aa631cc384f6b5c66c Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Wed, 6 Aug 2025 15:54:59 -0700 Subject: [PATCH 1/6] PYTHON-5488 `append_metadata` should not add duplicates --- pymongo/pool_options.py | 2 ++ test/asynchronous/test_client_metadata.py | 40 +++++++++++++++++++++++ test/test_client_metadata.py | 40 +++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 5c24709b16..c8ffb54fa3 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -386,6 +386,8 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata""" + if driver.name in self.__metadata["driver"]["name"].split("|"): + return metadata = copy.deepcopy(self.__metadata) if driver.name: diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index cfecb49748..5239b24a5c 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -210,6 +210,46 @@ async def test_doesnt_update_established_connections(self): self.assertIsNone(self.handshake_req) self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) + async def test_duplicate_driver_name_no_op(self): + client = await self.async_rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) + # send initial metadata + name, version, platform, metadata = await self.send_ping_and_get_metadata(client, True) + # wait for connection to become idle + await asyncio.sleep(0.005) + + # add new metadata + client.append_metadata(DriverInfo("framework", None, None)) + new_name, new_version, new_platform, new_metadata = await self.send_ping_and_get_metadata( + client, True + ) + self.assertEqual(new_name, f"{name}|framework") + self.assertEqual(new_version, version) + self.assertEqual(new_platform, platform) + + metadata.pop("driver") + metadata.pop("platform") + new_metadata.pop("driver") + new_metadata.pop("platform") + self.assertEqual(metadata, new_metadata) + + # wait for connection to become idle + await asyncio.sleep(0.005) + # add same metadata again + client.append_metadata(DriverInfo("framework", None, None)) + ( + same_name, + same_version, + same_platform, + same_metadata, + ) = await self.send_ping_and_get_metadata(client, True) + self.assertEqual(new_name, same_name) + self.assertEqual(new_version, same_version) + self.assertEqual(new_platform, same_platform) + if __name__ == "__main__": unittest.main() diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 32cb9b8009..13cfa45f11 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -210,6 +210,46 @@ def test_doesnt_update_established_connections(self): self.assertIsNone(self.handshake_req) self.assertEqual(listener.event_count(ConnectionClosedEvent), 0) + def test_duplicate_driver_name_no_op(self): + client = self.rs_or_single_client( + "mongodb://" + self.server.address_string, + maxIdleTimeMS=1, + ) + client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) + # send initial metadata + name, version, platform, metadata = self.send_ping_and_get_metadata(client, True) + # wait for connection to become idle + time.sleep(0.005) + + # add new metadata + client.append_metadata(DriverInfo("framework", None, None)) + new_name, new_version, new_platform, new_metadata = self.send_ping_and_get_metadata( + client, True + ) + self.assertEqual(new_name, f"{name}|framework") + self.assertEqual(new_version, version) + self.assertEqual(new_platform, platform) + + metadata.pop("driver") + metadata.pop("platform") + new_metadata.pop("driver") + new_metadata.pop("platform") + self.assertEqual(metadata, new_metadata) + + # wait for connection to become idle + time.sleep(0.005) + # add same metadata again + client.append_metadata(DriverInfo("framework", None, None)) + ( + same_name, + same_version, + same_platform, + same_metadata, + ) = self.send_ping_and_get_metadata(client, True) + self.assertEqual(new_name, same_name) + self.assertEqual(new_version, same_version) + self.assertEqual(new_platform, same_platform) + if __name__ == "__main__": unittest.main() From 69f5a6a5c3afbcc4e8e0f3cd66f049a5e143bead Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Thu, 7 Aug 2025 10:40:53 -0700 Subject: [PATCH 2/6] make it case insensitive --- pymongo/pool_options.py | 2 +- test/asynchronous/test_client_metadata.py | 2 +- test/test_client_metadata.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index c8ffb54fa3..13d35f135f 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -386,7 +386,7 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata""" - if driver.name in self.__metadata["driver"]["name"].split("|"): + if driver.name.lower() in self.__metadata["driver"]["name"].lower().split("|"): return metadata = copy.deepcopy(self.__metadata) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 5239b24a5c..552ccd332b 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -239,7 +239,7 @@ async def test_duplicate_driver_name_no_op(self): # wait for connection to become idle await asyncio.sleep(0.005) # add same metadata again - client.append_metadata(DriverInfo("framework", None, None)) + client.append_metadata(DriverInfo("Framework", None, None)) ( same_name, same_version, diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index 13cfa45f11..e660f41279 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -239,7 +239,7 @@ def test_duplicate_driver_name_no_op(self): # wait for connection to become idle time.sleep(0.005) # add same metadata again - client.append_metadata(DriverInfo("framework", None, None)) + client.append_metadata(DriverInfo("Framework", None, None)) ( same_name, same_version, From 5e47df34767483ce991e9564063316a48701aa10 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Fri, 8 Aug 2025 10:38:44 -0700 Subject: [PATCH 3/6] add none check and refactor test --- pymongo/pool_options.py | 4 +- test/asynchronous/test_client_metadata.py | 55 +++++++---------------- test/test_client_metadata.py | 55 +++++++---------------- 3 files changed, 35 insertions(+), 79 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 13d35f135f..f0c0a7c8f2 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -386,7 +386,9 @@ def __init__( def _update_metadata(self, driver: DriverInfo) -> None: """Updates the client's metadata""" - if driver.name.lower() in self.__metadata["driver"]["name"].lower().split("|"): + if driver.name and driver.name.lower() in self.__metadata["driver"]["name"].lower().split( + "|" + ): return metadata = copy.deepcopy(self.__metadata) diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 552ccd332b..2f175cceed 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -107,15 +107,20 @@ async def check_metadata_added( new_name, new_version, new_platform, new_metadata = await self.send_ping_and_get_metadata( client, True ) - 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, - ) + 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, + ) metadata.pop("driver") metadata.pop("platform") @@ -216,39 +221,11 @@ async def test_duplicate_driver_name_no_op(self): maxIdleTimeMS=1, ) client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) - # send initial metadata - name, version, platform, metadata = await self.send_ping_and_get_metadata(client, True) - # wait for connection to become idle - await asyncio.sleep(0.005) - - # add new metadata - client.append_metadata(DriverInfo("framework", None, None)) - new_name, new_version, new_platform, new_metadata = await self.send_ping_and_get_metadata( - client, True - ) - self.assertEqual(new_name, f"{name}|framework") - self.assertEqual(new_version, version) - self.assertEqual(new_platform, platform) - - metadata.pop("driver") - metadata.pop("platform") - new_metadata.pop("driver") - new_metadata.pop("platform") - self.assertEqual(metadata, new_metadata) - + await self.check_metadata_added(client, "framework", None, None) # wait for connection to become idle await asyncio.sleep(0.005) # add same metadata again - client.append_metadata(DriverInfo("Framework", None, None)) - ( - same_name, - same_version, - same_platform, - same_metadata, - ) = await self.send_ping_and_get_metadata(client, True) - self.assertEqual(new_name, same_name) - self.assertEqual(new_version, same_version) - self.assertEqual(new_platform, same_platform) + await self.check_metadata_added(client, "Framework", None, None) if __name__ == "__main__": diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index e660f41279..a94c5aa25e 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -107,15 +107,20 @@ def check_metadata_added( new_name, new_version, new_platform, new_metadata = self.send_ping_and_get_metadata( client, True ) - 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, - ) + 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, + ) metadata.pop("driver") metadata.pop("platform") @@ -216,39 +221,11 @@ def test_duplicate_driver_name_no_op(self): maxIdleTimeMS=1, ) client.append_metadata(DriverInfo("library", "1.2", "Library Platform")) - # send initial metadata - name, version, platform, metadata = self.send_ping_and_get_metadata(client, True) - # wait for connection to become idle - time.sleep(0.005) - - # add new metadata - client.append_metadata(DriverInfo("framework", None, None)) - new_name, new_version, new_platform, new_metadata = self.send_ping_and_get_metadata( - client, True - ) - self.assertEqual(new_name, f"{name}|framework") - self.assertEqual(new_version, version) - self.assertEqual(new_platform, platform) - - metadata.pop("driver") - metadata.pop("platform") - new_metadata.pop("driver") - new_metadata.pop("platform") - self.assertEqual(metadata, new_metadata) - + self.check_metadata_added(client, "framework", None, None) # wait for connection to become idle time.sleep(0.005) # add same metadata again - client.append_metadata(DriverInfo("Framework", None, None)) - ( - same_name, - same_version, - same_platform, - same_metadata, - ) = self.send_ping_and_get_metadata(client, True) - self.assertEqual(new_name, same_name) - self.assertEqual(new_version, same_version) - self.assertEqual(new_platform, same_platform) + self.check_metadata_added(client, "Framework", None, None) if __name__ == "__main__": From 30f4ac123046afa2ab3832997b8dd0f044f19ce7 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Mon, 11 Aug 2025 09:20:05 -0700 Subject: [PATCH 4/6] always add | --- pymongo/pool_options.py | 26 ++++++++++++----------- test/asynchronous/test_client_metadata.py | 6 +++--- test/test_client_metadata.py | 6 +++--- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index f0c0a7c8f2..92f8262783 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -392,18 +392,20 @@ def _update_metadata(self, driver: DriverInfo) -> None: 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, - ) - if driver.platform: - metadata["platform"] = "{}|{}".format(metadata["platform"], driver.platform) + + name = driver.name if driver.name else "" + version = driver.version if driver.version else "" + platform = driver.platform if driver.platform else "" + + metadata["driver"]["name"] = "{}|{}".format( + metadata["driver"]["name"], + name, + ) + metadata["driver"]["version"] = "{}|{}".format( + metadata["driver"]["version"], + version, + ) + metadata["platform"] = "{}|{}".format(metadata["platform"], platform) self.__metadata = metadata diff --git a/test/asynchronous/test_client_metadata.py b/test/asynchronous/test_client_metadata.py index 2f175cceed..18eb72b5bb 100644 --- a/test/asynchronous/test_client_metadata.py +++ b/test/asynchronous/test_client_metadata.py @@ -112,14 +112,14 @@ async def check_metadata_added( 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_name, f"{name}|{add_name}" if add_name is not None else f"{name}|") self.assertEqual( new_version, - f"{version}|{add_version}" if add_version is not None else version, + f"{version}|{add_version}" if add_version is not None else f"{version}|", ) self.assertEqual( new_platform, - f"{platform}|{add_platform}" if add_platform is not None else platform, + f"{platform}|{add_platform}" if add_platform is not None else f"{platform}|", ) metadata.pop("driver") diff --git a/test/test_client_metadata.py b/test/test_client_metadata.py index a94c5aa25e..5e1822d755 100644 --- a/test/test_client_metadata.py +++ b/test/test_client_metadata.py @@ -112,14 +112,14 @@ def check_metadata_added( 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_name, f"{name}|{add_name}" if add_name is not None else f"{name}|") self.assertEqual( new_version, - f"{version}|{add_version}" if add_version is not None else version, + f"{version}|{add_version}" if add_version is not None else f"{version}|", ) self.assertEqual( new_platform, - f"{platform}|{add_platform}" if add_platform is not None else platform, + f"{platform}|{add_platform}" if add_platform is not None else f"{platform}|", ) metadata.pop("driver") From c469174e55cd04881dd19fd0f96b02d4ab0acffd Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Mon, 11 Aug 2025 09:50:21 -0700 Subject: [PATCH 5/6] fix test --- test/asynchronous/test_client.py | 1 + test/test_client.py | 1 + 2 files changed, 2 insertions(+) diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 52f16e0bcc..3a4c804220 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -372,6 +372,7 @@ async def test_metadata(self): else: metadata["driver"]["name"] = "PyMongo|async|FooDriver" metadata["driver"]["version"] = "{}|1.2.3".format(_METADATA["driver"]["version"]) + metadata["platform"] = "{}|".format(_METADATA["platform"]) client = self.simple_client( "foo", 27017, diff --git a/test/test_client.py b/test/test_client.py index dd1bf94cf1..1c2ebcf13c 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -365,6 +365,7 @@ def test_metadata(self): else: metadata["driver"]["name"] = "PyMongo|FooDriver" metadata["driver"]["version"] = "{}|1.2.3".format(_METADATA["driver"]["version"]) + metadata["platform"] = "{}|".format(_METADATA["platform"]) client = self.simple_client( "foo", 27017, From b5314ab9e8f79991f315099954a2a3a22aff2315 Mon Sep 17 00:00:00 2001 From: Iris Ho Date: Mon, 11 Aug 2025 10:50:53 -0700 Subject: [PATCH 6/6] simplify logic --- pymongo/pool_options.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pymongo/pool_options.py b/pymongo/pool_options.py index 92f8262783..b6c1071f62 100644 --- a/pymongo/pool_options.py +++ b/pymongo/pool_options.py @@ -393,9 +393,9 @@ def _update_metadata(self, driver: DriverInfo) -> None: metadata = copy.deepcopy(self.__metadata) - name = driver.name if driver.name else "" - version = driver.version if driver.version else "" - platform = driver.platform if driver.platform else "" + name = driver.name or "" + version = driver.version or "" + platform = driver.platform or "" metadata["driver"]["name"] = "{}|{}".format( metadata["driver"]["name"],