Rev 6405: (jelmer) Add a post_connect hook for transports. (Bazaar Developers) in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

Patch Queue Manager pqm at pqm.ubuntu.com
Sat Dec 24 09:59:05 UTC 2011


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6405 [merge]
revision-id: pqm at pqm.ubuntu.com-20111224095902-kkoi2q9k7cc4elxt
parent: pqm at pqm.ubuntu.com-20111222185258-wgcba8590pbw5sf1
parent: martin.packman at canonical.com-20111223193822-hesheea4o8aqwexv
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Sat 2011-12-24 09:59:02 +0000
message:
  (jelmer) Add a post_connect hook for transports. (Bazaar Developers)
modified:
  bzrlib/hooks.py                hooks.py-20070325015548-ix4np2q0kd8452au-1
  bzrlib/smart/medium.py         medium.py-20061103051856-rgu2huy59fkz902q-1
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/per_transport.py  test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
  bzrlib/tests/transport_util.py transportutil.py-20070525113600-5v2igk89s8fensom-1
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
=== modified file 'bzrlib/hooks.py'
--- a/bzrlib/hooks.py	2011-12-19 13:23:58 +0000
+++ b/bzrlib/hooks.py	2011-12-24 09:59:02 +0000
@@ -83,6 +83,7 @@
     ('bzrlib.smart.client', '_SmartClient.hooks', 'SmartClientHooks'),
     ('bzrlib.smart.server', 'SmartTCPServer.hooks', 'SmartServerHooks'),
     ('bzrlib.status', 'hooks', 'StatusHooks'),
+    ('bzrlib.transport', 'Transport.hooks', 'TransportHooks'),
     ('bzrlib.version_info_formats.format_rio', 'RioVersionInfoBuilder.hooks',
         'RioVersionInfoBuilderHooks'),
     ('bzrlib.merge_directive', 'BaseMergeDirective.hooks',

=== modified file 'bzrlib/smart/medium.py'
--- a/bzrlib/smart/medium.py	2011-12-19 13:23:58 +0000
+++ b/bzrlib/smart/medium.py	2011-12-24 09:59:02 +0000
@@ -43,6 +43,7 @@
     debug,
     errors,
     trace,
+    transport,
     ui,
     urlutils,
     )
@@ -1021,6 +1022,8 @@
             raise AssertionError(
                 "Unexpected io_kind %r from %r"
                 % (io_kind, self._ssh_connection))
+        for hook in transport.Transport.hooks["post_connect"]:
+            hook(self)
 
     def _flush(self):
         """See SmartClientStreamMedium._flush()."""
@@ -1129,6 +1132,8 @@
             raise errors.ConnectionError("failed to connect to %s:%d: %s" %
                     (self._host, port, err_msg))
         self._connected = True
+        for hook in transport.Transport.hooks["post_connect"]:
+            hook(self)
 
 
 class SmartClientAlreadyConnectedSocketMedium(SmartClientSocketMedium):

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2011-12-18 12:46:49 +0000
+++ b/bzrlib/tests/__init__.py	2011-12-24 09:59:02 +0000
@@ -2734,16 +2734,20 @@
 
     def setUp(self):
         super(TestCaseWithMemoryTransport, self).setUp()
-        # Ensure that ConnectedTransport doesn't leak sockets
-        def get_transport_from_url_with_cleanup(*args, **kwargs):
-            t = orig_get_transport_from_url(*args, **kwargs)
-            if isinstance(t, _mod_transport.ConnectedTransport):
-                self.addCleanup(t.disconnect)
-            return t
-
-        orig_get_transport_from_url = self.overrideAttr(
-            _mod_transport, 'get_transport_from_url',
-            get_transport_from_url_with_cleanup)
+
+        def _add_disconnect_cleanup(transport):
+            """Schedule disconnection of given transport at test cleanup
+
+            This needs to happen for all connected transports or leaks occur.
+
+            Note reconnections may mean we call disconnect multiple times per
+            transport which is suboptimal but seems harmless.
+            """
+            self.addCleanup(transport.disconnect)
+ 
+        _mod_transport.Transport.hooks.install_named_hook('post_connect',
+            _add_disconnect_cleanup, None)
+
         self._make_test_root()
         self.addCleanup(os.chdir, os.getcwdu())
         self.makeAndChdirToTestDir()

=== modified file 'bzrlib/tests/per_transport.py'
--- a/bzrlib/tests/per_transport.py	2011-11-17 18:06:46 +0000
+++ b/bzrlib/tests/per_transport.py	2011-12-23 19:38:22 +0000
@@ -53,9 +53,11 @@
 from bzrlib.tests.test_transport import TestTransportImplementation
 from bzrlib.transport import (
     ConnectedTransport,
+    Transport,
     _get_transport_modules,
     )
 from bzrlib.transport.memory import MemoryTransport
+from bzrlib.transport.remote import RemoteTransport
 
 
 def get_transport_test_permutations(module):
@@ -1836,3 +1838,35 @@
         self.build_tree([needlessly_escaped_dir], transport=t1)
         t2 = t1.clone(needlessly_escaped_dir)
         self.assertEqual(t1.base + "-.09AZ_az~/", t2.base)
+
+    def test_hook_post_connection_one(self):
+        """Fire post_connect hook after a ConnectedTransport is first used"""
+        log = []
+        Transport.hooks.install_named_hook("post_connect", log.append, None)
+        t = self.get_transport()
+        self.assertEqual([], log)
+        t.has("non-existant")
+        if isinstance(t, RemoteTransport):
+            self.assertEqual([t.get_smart_medium()], log)
+        elif isinstance(t, ConnectedTransport):
+            self.assertEqual([t], log)
+        else:
+            self.assertEqual([], log)
+
+    def test_hook_post_connection_multi(self):
+        """Fire post_connect hook once per unshared underlying connection"""
+        log = []
+        Transport.hooks.install_named_hook("post_connect", log.append, None)
+        t1 = self.get_transport()
+        t2 = t1.clone(".")
+        t3 = self.get_transport()
+        self.assertEqual([], log)
+        t1.has("x")
+        t2.has("x")
+        t3.has("x")
+        if isinstance(t1, RemoteTransport):
+            self.assertEqual([t.get_smart_medium() for t in [t1, t3]], log)
+        elif isinstance(t1, ConnectedTransport):
+            self.assertEqual([t1, t3], log)
+        else:
+            self.assertEqual([], log)

=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py	2011-12-05 14:21:55 +0000
+++ b/bzrlib/tests/test_transport.py	2011-12-14 19:44:40 +0000
@@ -443,6 +443,29 @@
         self.assertEqual('chroot-%d:///' % id(server), server.get_url())
 
 
+class TestHooks(tests.TestCase):
+    """Basic tests for transport hooks"""
+
+    def _get_connected_transport(self):
+        return transport.ConnectedTransport("bogus:nowhere")
+
+    def test_transporthooks_initialisation(self):
+        """Check all expected transport hook points are set up"""
+        hookpoint = transport.TransportHooks()
+        self.assertTrue("post_connect" in hookpoint,
+            "post_connect not in %s" % (hookpoint,))
+
+    def test_post_connect(self):
+        """Ensure the post_connect hook is called when _set_transport is"""
+        calls = []
+        transport.Transport.hooks.install_named_hook("post_connect",
+            calls.append, None)
+        t = self._get_connected_transport()
+        self.assertLength(0, calls)
+        t._set_connection("connection", "auth")
+        self.assertEqual(calls, [t])
+
+
 class PathFilteringDecoratorTransportTest(tests.TestCase):
     """Pathfilter decoration specific tests."""
 

=== modified file 'bzrlib/tests/transport_util.py'
--- a/bzrlib/tests/transport_util.py	2011-08-19 22:34:02 +0000
+++ b/bzrlib/tests/transport_util.py	2011-12-14 19:44:40 +0000
@@ -14,125 +14,34 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
-import bzrlib.hooks
-from bzrlib import transport
 from bzrlib.tests import features
 
 # SFTPTransport offers better performances but relies on paramiko, if paramiko
 # is not available, we fallback to FtpTransport
 if features.paramiko.available():
     from bzrlib.tests import test_sftp_transport
-    from bzrlib.transport import sftp
+    from bzrlib.transport import sftp, Transport
     _backing_scheme = 'sftp'
     _backing_transport_class = sftp.SFTPTransport
     _backing_test_class = test_sftp_transport.TestCaseWithSFTPServer
 else:
-    from bzrlib.transport import ftp
+    from bzrlib.transport import ftp, Transport
     from bzrlib.tests import test_ftp_transport
     _backing_scheme = 'ftp'
     _backing_transport_class = ftp.FtpTransport
     _backing_test_class = test_ftp_transport.TestCaseWithFTPServer
 
-from bzrlib.transport import (
-    ConnectedTransport,
-    register_transport,
-    register_urlparse_netloc_protocol,
-    unregister_transport,
-    _unregister_urlparse_netloc_protocol,
-    )
-
-
-
-class TransportHooks(bzrlib.hooks.Hooks):
-    """Dict-mapping hook name to a list of callables for transport hooks"""
-
-    def __init__(self):
-        super(TransportHooks, self).__init__("bzrlib.tests.transport_util",
-            "InstrumentedTransport.hooks")
-        # Invoked when the transport has just created a new connection.
-        # The api signature is (transport, connection, credentials)
-        self['_set_connection'] = []
-
-_hooked_scheme = 'hooked'
-
-def _change_scheme_in(url, actual, desired):
-    if not url.startswith(actual + '://'):
-        raise AssertionError('url "%r" does not start with "%r]"'
-                             % (url, actual))
-    return desired + url[len(actual):]
-
-
-class InstrumentedTransport(_backing_transport_class):
-    """Instrumented transport class to test commands behavior"""
-
-    hooks = TransportHooks()
-
-    def __init__(self, base, _from_transport=None):
-        if not base.startswith(_hooked_scheme + '://'):
-            raise ValueError(base)
-        # We need to trick the backing transport class about the scheme used
-        # We'll do the reverse when we need to talk to the backing server
-        fake_base = _change_scheme_in(base, _hooked_scheme, _backing_scheme)
-        super(InstrumentedTransport, self).__init__(
-            fake_base, _from_transport=_from_transport)
-        # The following is needed to minimize the effects of our trick above
-        # while retaining the best compatibility.
-        self._parsed_url.scheme = _hooked_scheme
-        super(ConnectedTransport, self).__init__(str(self._parsed_url))
-
-
-class ConnectionHookedTransport(InstrumentedTransport):
-    """Transport instrumented to inspect connections"""
-
-    def _set_connection(self, connection, credentials):
-        """Called when a new connection is created """
-        super(ConnectionHookedTransport, self)._set_connection(connection,
-                                                               credentials)
-        for hook in self.hooks['_set_connection']:
-            hook(self, connection, credentials)
-
 
 class TestCaseWithConnectionHookedTransport(_backing_test_class):
 
     def setUp(self):
-        register_urlparse_netloc_protocol(_hooked_scheme)
-        register_transport(_hooked_scheme, ConnectionHookedTransport)
-        self.addCleanup(unregister_transport, _hooked_scheme,
-                        ConnectionHookedTransport)
-        self.addCleanup(_unregister_urlparse_netloc_protocol, _hooked_scheme)
         super(TestCaseWithConnectionHookedTransport, self).setUp()
         self.reset_connections()
-        # Add the 'hooked' url to the permitted url list.
-        # XXX: See TestCase.start_server. This whole module shouldn't need to
-        # exist - a bug has been filed on that. once its cleanedup/removed, the
-        # standard test support code will work and permit the server url
-        # correctly.
-        url = self.get_url()
-        t = transport.get_transport_from_url(url)
-        if t.base.endswith('work/'):
-            t = t.clone('../..')
-        self.permit_url(t.base)
-
-    def get_url(self, relpath=None):
-        super_self = super(TestCaseWithConnectionHookedTransport, self)
-        url = super_self.get_url(relpath)
-        # Replace the backing scheme by our own (see
-        # InstrumentedTransport.__init__)
-        url = _change_scheme_in(url, _backing_scheme, _hooked_scheme)
-        return url
 
     def start_logging_connections(self):
-        self.overrideAttr(InstrumentedTransport, 'hooks', TransportHooks())
-        # We preserved the hooks class attribute. Now we install our hook.
-        ConnectionHookedTransport.hooks.install_named_hook(
-            '_set_connection', self._collect_connection, None)
+        Transport.hooks.install_named_hook('post_connect',
+            self.connections.append, None)
 
     def reset_connections(self):
         self.connections = []
 
-    def _collect_connection(self, transport, connection, credentials):
-        # Note: uncomment the following line and use 'bt' under pdb, that will
-        # identify all the connections made including the extraneous ones.
-        # import pdb; pdb.set_trace()
-        self.connections.append(connection)
-

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2011-12-19 10:58:39 +0000
+++ b/bzrlib/transport/__init__.py	2011-12-24 09:59:02 +0000
@@ -52,7 +52,10 @@
 from bzrlib.trace import (
     mutter,
     )
-from bzrlib import registry
+from bzrlib import (
+    hooks,
+    registry,
+    )
 
 
 # a dictionary of open file streams. Keys are absolute paths, values are
@@ -283,6 +286,16 @@
         self.transport.append_bytes(self.relpath, bytes)
 
 
+class TransportHooks(hooks.Hooks):
+    """Mapping of hook names to registered callbacks for transport hooks"""
+    def __init__(self):
+        super(TransportHooks, self).__init__()
+        self.add_hook("post_connect",
+            "Called after a new connection is established or a reconnect "
+            "occurs. The sole argument passed is either the connected "
+            "transport or smart medium instance.", (2, 5))
+
+
 class Transport(object):
     """This class encapsulates methods for retrieving or putting a file
     from/to a storage location.
@@ -307,6 +320,8 @@
     #       where the biggest benefit between combining reads and
     #       and seeking is. Consider a runtime auto-tune.
     _bytes_to_read_before_seek = 0
+    
+    hooks = TransportHooks()
 
     def __init__(self, base):
         super(Transport, self).__init__()
@@ -1496,6 +1511,8 @@
         """
         self._shared_connection.connection = connection
         self._shared_connection.credentials = credentials
+        for hook in self.hooks["post_connect"]:
+            hook(self)
 
     def _get_connection(self):
         """Returns the transport specific connection object."""




More information about the bazaar-commits mailing list