PYTHON-2981 Stop using MongoClient.address for hashing and equality (#795)

Author: ShaneHarvey

doc/changelog.rst

@@ -177,6 +177,9 @@ Breaking Changes in 4.0
   :exc:`~pymongo.errors.InvalidURI` exception
   when it encounters unescaped percent signs in username and password when
   parsing MongoDB URIs.
+- Comparing two :class:`~pymongo.mongo_client.MongoClient` instances now
+  uses a set of immutable properties rather than
+  :attr:`~pymongo.mongo_client.MongoClient.address` which can change.
 - Removed the `disable_md5` parameter for :class:`~gridfs.GridFSBucket` and
   :class:`~gridfs.GridFS`. See :ref:`removed-gridfs-checksum` for details.
 - PyMongoCrypt 1.2.0 or later is now required for client side field level

pymongo/mongo_client.py

@@ -748,9 +748,9 @@ def __init__(
             server_selector=options.server_selector,
             heartbeat_frequency=options.heartbeat_frequency,
             fqdn=fqdn,
-            srv_service_name=srv_service_name,
             direct_connection=options.direct_connection,
             load_balanced=options.load_balanced,
+            srv_service_name=srv_service_name,
             srv_max_hosts=srv_max_hosts
         )
 
@@ -1337,14 +1337,14 @@ def _retryable_write(self, retryable, func, session):
 
     def __eq__(self, other):
         if isinstance(other, self.__class__):
-            return self.address == other.address
+            return self._topology == other._topology
         return NotImplemented
 
     def __ne__(self, other):
         return not self == other
 
     def __hash__(self):
-        return hash(self.address)
+        return hash(self._topology)
 
     def _repr_helper(self):
         def option_repr(option, value):

pymongo/monitor.py

@@ -299,7 +299,6 @@ def __init__(self, topology, topology_settings):
         self._settings = topology_settings
         self._seedlist = self._settings._seeds
         self._fqdn = self._settings.fqdn
-        self._srv_service_name = self._settings._srv_service_name
 
     def _run(self):
         seedlist = self._get_seedlist()
@@ -319,7 +318,7 @@ def _get_seedlist(self):
         try:
             resolver = _SrvResolver(self._fqdn,
                                     self._settings.pool_options.connect_timeout,
-                                    self._srv_service_name)
+                                    self._settings.srv_service_name)
             seedlist, ttl = resolver.get_hosts_and_min_ttl()
             if len(seedlist) == 0:
                 # As per the spec: this should be treated as a failure.

pymongo/settings.py

@@ -39,9 +39,9 @@ def __init__(self,
                  heartbeat_frequency=common.HEARTBEAT_FREQUENCY,
                  server_selector=None,
                  fqdn=None,
-                 srv_service_name=common.SRV_SERVICE_NAME,
                  direct_connection=False,
                  load_balanced=None,
+                 srv_service_name=common.SRV_SERVICE_NAME,
                  srv_max_hosts=0):
         """Represent MongoClient's configuration.
 
@@ -62,11 +62,11 @@ def __init__(self,
         self._server_selection_timeout = server_selection_timeout
         self._server_selector = server_selector
         self._fqdn = fqdn
-        self._srv_service_name = srv_service_name
         self._heartbeat_frequency = heartbeat_frequency
-        self._srv_max_hosts = srv_max_hosts or 0
         self._direct = direct_connection
         self._load_balanced = load_balanced
+        self._srv_service_name = srv_service_name
+        self._srv_max_hosts = srv_max_hosts or 0
 
         self._topology_id = ObjectId()
         # Store the allocation traceback to catch unclosed clients in the
@@ -131,6 +131,16 @@ def load_balanced(self):
         """True if the client was configured to connect to a load balancer."""
         return self._load_balanced
 
+    @property
+    def srv_service_name(self):
+        """The srvServiceName."""
+        return self._srv_service_name
+
+    @property
+    def srv_max_hosts(self):
+        """The srvMaxHosts."""
+        return self._srv_max_hosts
+
     def get_topology_type(self):
         if self.load_balanced:
             return TOPOLOGY_TYPE.LoadBalanced

pymongo/topology.py

@@ -803,6 +803,20 @@ def __repr__(self):
             msg = 'CLOSED '
         return '<%s %s%r>' % (self.__class__.__name__, msg, self._description)
 
+    def eq_props(self):
+        """The properties to use for MongoClient/Topology equality checks."""
+        ts = self._settings
+        return (tuple(sorted(ts.seeds)), ts.replica_set_name, ts.fqdn,
+                ts.srv_service_name)
+
+    def __eq__(self, other):
+        if isinstance(other, self.__class__):
+            return self.eq_props() == other.eq_props()
+        return NotImplemented
+
+    def __hash__(self):
+        return hash(self.eq_props())
+
 
 class _ErrorContext(object):
     """An error with context for SDAM error handling."""

test/test_client.py

@@ -676,15 +676,32 @@ def test_init_disconnected_with_auth(self):
         self.assertRaises(ConnectionFailure, c.pymongo_test.test.find_one)
 
     def test_equality(self):
-        c = connected(rs_or_single_client())
+        seed = '%s:%s' % list(self.client._topology_settings.seeds)[0]
+        c = rs_or_single_client(seed, connect=False)
+        self.addCleanup(c.close)
         self.assertEqual(client_context.client, c)
-
         # Explicitly test inequality
         self.assertFalse(client_context.client != c)
 
+        c = rs_or_single_client('invalid.com', connect=False)
+        self.addCleanup(c.close)
+        self.assertNotEqual(client_context.client, c)
+        self.assertTrue(client_context.client != c)
+        # Seeds differ:
+        self.assertNotEqual(MongoClient('a', connect=False),
+                            MongoClient('b', connect=False))
+        # Same seeds but out of order still compares equal:
+        self.assertEqual(MongoClient(['a', 'b', 'c'], connect=False),
+                         MongoClient(['c', 'a', 'b'], connect=False))
+
     def test_hashable(self):
-        c = connected(rs_or_single_client())
+        seed = '%s:%s' % list(self.client._topology_settings.seeds)[0]
+        c = rs_or_single_client(seed, connect=False)
+        self.addCleanup(c.close)
         self.assertIn(c, {client_context.client})
+        c = rs_or_single_client('invalid.com', connect=False)
+        self.addCleanup(c.close)
+        self.assertNotIn(c, {client_context.client})
 
     def test_host_w_port(self):
         with self.assertRaises(ValueError):
@@ -1635,19 +1652,19 @@ def test_service_name_from_kwargs(self):
         client = MongoClient(
             'mongodb+srv://user:password@test22.test.build.10gen.cc',
             srvServiceName='customname', connect=False)
-        self.assertEqual(client._topology_settings._srv_service_name,
+        self.assertEqual(client._topology_settings.srv_service_name,
                          'customname')
         client = MongoClient(
             'mongodb+srv://user:password@test22.test.build.10gen.cc'
             '/?srvServiceName=shouldbeoverriden',
             srvServiceName='customname', connect=False)
-        self.assertEqual(client._topology_settings._srv_service_name,
+        self.assertEqual(client._topology_settings.srv_service_name,
                          'customname')
         client = MongoClient(
              'mongodb+srv://user:password@test22.test.build.10gen.cc'
              '/?srvServiceName=customname',
              connect=False)
-        self.assertEqual(client._topology_settings._srv_service_name,
+        self.assertEqual(client._topology_settings.srv_service_name,
                          'customname')
 
     @unittest.skipUnless(