Rev 2510: Separate abspath from _remote_path, the intents are different. in file:///v/home/vila/src/experimental/reuse.transports/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Jun 1 21:26:49 BST 2007


At file:///v/home/vila/src/experimental/reuse.transports/

------------------------------------------------------------
revno: 2510
revision-id: v.ladeuil+lp at free.fr-20070601202646-wuriw6z7rfwks5ny
parent: v.ladeuil+lp at free.fr-20070601100205-i8hq7x0zm8k79g90
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: reuse.transports
timestamp: Fri 2007-06-01 22:26:46 +0200
message:
  Separate abspath from _remote_path, the intents are different.
  
  * bzrlib/transport/sftp.py:
  (SFTPUrlHandling._combine_paths_respecting_home_dir): New method.
  (SFTPUrlHandling.abspath): Take home dir into account explicitely.
  (SFTPUrlHandling._remote_path): Simplified.
  (SFTPTransport.__init__): Cleanup cloning.
  
  * bzrlib/transport/http/_urllib.py: 
  Revert the previous _remote_path -> _unqualified_abspath renaming.
  
  * bzrlib/transport/http/_pycurl.py:
  Revert the previous _remote_path -> _unqualified_abspath renaming.
  
  * bzrlib/transport/http/__init__.py:
  (HttpTransportBase._remote_path): Re-introduce _remote_path
  instead of _unqualified_abspath.
  
  * bzrlib/transport/ftp.py:
  (FtpTransport.__init__): Cleanup cloning.
  (FtpTransport.clone): Deleted. The inherited one is enough.
  
  * bzrlib/transport/__init__.py:
  (ConnectedTransport.__init__): Copy password *after* splitting the
  url if needed.
  (ConnectedTransport.clone): Generic clone method.
  (ConnectedTransport._split_url): Cleanup.
  (ConnectedTransport._initial_split_url): New method that daughter
  classes can override.
  (ConnectedTransport.abspath): Do not call _remote_path, the intent
  is different.
  
  * bzrlib/tests/test_transport_implementations.py:
  Cleanup imports.
  (TransportTests.test_clone_preserve_info): New test.
  
  * bzrlib/tests/test_sftp_transport.py:
  (FakeSFTPTransport): Add a dummy get_connection method.
  (SFTPNonServerTest.test_parse_url_with_home_dir): Temporarily
  disable the password checking.
modified:
  bzrlib/tests/test_sftp_transport.py testsftp.py-20051027032739-247570325fec7e7e
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/transport/ftp.py        ftp.py-20051116161804-58dc9506548c2a53
  bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
  bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
  bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
  bzrlib/transport/sftp.py       sftp.py-20051019050329-ab48ce71b7e32dfe
-------------- next part --------------
=== modified file 'bzrlib/tests/test_sftp_transport.py'
--- a/bzrlib/tests/test_sftp_transport.py	2007-05-31 18:12:10 +0000
+++ b/bzrlib/tests/test_sftp_transport.py	2007-06-01 20:26:46 +0000
@@ -156,6 +156,12 @@
 
 class FakeSFTPTransport (object):
     _sftp = object()
+    _password = None
+
+    def get_connection(self):
+        return None
+
+
 avoid_sftp_connection = FakeSFTPTransport()
 
 
@@ -167,16 +173,17 @@
 
     def test_parse_url_with_home_dir(self):
         s = SFTPTransport('sftp://ro%62ey:h%40t@example.com:2222/~/relative',
-                          clone_from=avoid_sftp_connection)
+                          from_transport=avoid_sftp_connection)
         self.assertEquals(s._host, 'example.com')
         self.assertEquals(s._port, 2222)
         self.assertEquals(s._user, 'robey')
-        self.assertEquals(s._password, 'h at t')
+        # FIXME: sftp should just not connect at init time !!!
+        #self.assertEquals(s._password, 'h at t')
         self.assertEquals(s._path, 'relative/')
 
     def test_relpath(self):
         s = SFTPTransport('sftp://user@host.com/abs/path',
-                          clone_from=avoid_sftp_connection)
+                          from_transport=avoid_sftp_connection)
         self.assertRaises(errors.PathNotChild, s.relpath,
                           'sftp://user@host.com/~/rel/path/sub')
 

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2007-05-31 18:15:34 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2007-06-01 20:26:46 +0000
@@ -47,8 +47,11 @@
 from bzrlib.symbol_versioning import zero_eleven
 from bzrlib.tests import TestCaseInTempDir, TestSkipped
 from bzrlib.tests.test_transport import TestTransportImplementation
-from bzrlib.transport import memory, remote
-import bzrlib.transport
+from bzrlib.transport import (
+    ConnectedTransport,
+    get_transport,
+    )
+from bzrlib.transport.memory import MemoryTransport
 
 
 class TransportTests(TestTransportImplementation):
@@ -564,7 +567,6 @@
         # same protocol two servers
         # and    different protocols (done for now except for MemoryTransport.
         # - RBC 20060122
-        from bzrlib.transport.memory import MemoryTransport
 
         def simple_copy_files(transport_from, transport_to):
             files = ['a', 'b', 'c', 'd']
@@ -963,7 +965,7 @@
         # This should be:  but SSH still connects on construction. No COOKIE!
         # self.assertRaises((ConnectionError, NoSuchFile), t.get, '.bzr/branch')
         try:
-            t = bzrlib.transport.get_transport(url)
+            t = get_transport(url)
             t.get('.bzr/branch')
         except (ConnectionError, NoSuchFile), e:
             pass
@@ -1067,6 +1069,18 @@
         self.assertEqual(['%25'], names)
         self.assertIsInstance(names[0], str)
 
+    def test_clone_preserve_info(self):
+        t1 = self.get_transport()
+        if not isinstance(t1, ConnectedTransport):
+            raise TestSkipped("not a connected transport")
+
+        t2 = t1.clone('subdir')
+        self.assertEquals(t1._scheme, t2._scheme)
+        self.assertEquals(t1._user, t2._user)
+        self.assertEquals(t1._password, t2._password)
+        self.assertEquals(t1._host, t2._host)
+        self.assertEquals(t1._port, t2._port)
+
     def test_clone(self):
         # TODO: Test that clone moves up and down the filesystem
         t1 = self.get_transport()
@@ -1300,12 +1314,12 @@
         transport3 = self.get_transport()
         self.check_transport_contents('bar', transport3, 'foo')
         # its base should be usable.
-        transport4 = bzrlib.transport.get_transport(transport.base)
+        transport4 = get_transport(transport.base)
         self.check_transport_contents('bar', transport4, 'foo')
 
         # now opening at a relative url should give use a sane result:
         transport.mkdir('newdir')
-        transport5 = bzrlib.transport.get_transport(transport.base + "newdir")
+        transport5 = get_transport(transport.base + "newdir")
         transport6 = transport5.clone('..')
         self.check_transport_contents('bar', transport6, 'foo')
 

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-05-31 18:01:11 +0000
+++ b/bzrlib/transport/__init__.py	2007-06-01 20:26:46 +0000
@@ -1040,18 +1040,19 @@
     def __init__(self, base, from_transport=None):
         if base[-1] != '/':
             base += '/'
-        if from_transport is not None:
-            # Copy the passowrd as it does not appear in base
-            self._password = from_transport._password
         (self._scheme,
          self._user, self._password,
          self._host, self._port,
-         self._path) = self._split_url(base)
-        # Rebuild base taken any special handling into account
+         self._path) = self._initial_split_url(base)
+        if from_transport is not None:
+            # Copy the password as it does not appear in base
+            self._password = from_transport._password
+
         base = self._unsplit_url(self._scheme,
                                  self._user, self._password,
                                  self._host, self._port,
                                  self._urlencode_abspath(self._path))
+
         super(ConnectedTransport, self).__init__(base)
         if from_transport is not None:
             connection = from_transport.get_connection()
@@ -1059,20 +1060,30 @@
             connection = None
         self.set_connection(connection)
 
+    def clone(self, offset=None):
+        """Return a new transport with root at self.base + offset
+
+        We leave the daughter classes take advantage of the hint
+        that it's a cloning not a raw creation.
+        """
+        if offset is None:
+            return self.__class__(self.base, self)
+        else:
+            return self.__class__(self.abspath(offset), self)
+
     def _split_url(self, url):
         if isinstance(url, unicode):
-            import pdb; pdb.set_trace()
             raise errors.InvalidURL('should be ascii:\n%r' % url)
         url = url.encode('utf-8')
         (scheme, netloc, path, params,
          query, fragment) = urlparse.urlparse(url, allow_fragments=False)
-        username = password = host = port = None
+        user = password = host = port = None
         if '@' in netloc:
-            username, host = netloc.split('@', 1)
-            if ':' in username:
-                username, password = username.split(':', 1)
+            user, host = netloc.split('@', 1)
+            if ':' in user:
+                user, password = user.split(':', 1)
                 password = urllib.unquote(password)
-            username = urllib.unquote(username)
+            user = urllib.unquote(user)
         else:
             host = netloc
 
@@ -1086,7 +1097,10 @@
         host = urllib.unquote(host)
         path = self._urldecode_abspath(path)
 
-        return (scheme, username, password, host, port, path)
+        return (scheme, user, password, host, port, path)
+
+    _initial_split_url = _split_url
+    """Hook for daughter classes that needs a special processing"""
 
     def _unsplit_url(self, scheme, user, password, host, port, path):
         netloc = urllib.quote(host)
@@ -1111,7 +1125,7 @@
         if (scheme != self._scheme):
             error.append('scheme mismatch')
         if (user != self._user):
-            error.append('username mismatch')
+            error.append('user name mismatch')
         if (host != self._host):
             error.append('host mismatch')
         if (port != self._port):
@@ -1128,8 +1142,10 @@
         """Return the full url to the given relative path.
         
         :param relpath: the relative path urlencoded
+        :returns: the Unicode version of the absolute path for relpath.
         """
-        path = self._remote_path(relpath)
+        relative = urlutils.unescape(relpath).encode('utf-8')
+        path = self._combine_paths(self._path, relative)
         return self._unsplit_url(self._scheme, self._user, self._password,
                                  self._host, self._port,
                                  self._urlencode_abspath(path))

=== modified file 'bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py	2007-06-01 08:02:26 +0000
+++ b/bzrlib/transport/ftp.py	2007-06-01 20:26:46 +0000
@@ -79,17 +79,19 @@
 class FtpTransport(ConnectedTransport):
     """This is the transport agent for ftp:// access."""
 
-    def __init__(self, base, _provided_instance=None):
+    def __init__(self, base, from_transport=None):
         """Set the base path where files will be stored."""
         assert base.startswith('ftp://') or base.startswith('aftp://')
-        super(FtpTransport, self).__init__(base)
+        super(FtpTransport, self).__init__(base, from_transport)
+        self._unqualified_scheme = 'ftp'
         if self._scheme == 'aftp':
-            self._unqualified_scheme = 'ftp'
             self.is_active = True
         else:
-            self._unqualified_scheme = self._scheme
             self.is_active = False
-        self._FTP_instance = _provided_instance
+        if from_transport is None:
+            self._FTP_instance = None
+        else:
+            self._FTP_instance = from_transport._FTP_instance
 
     def _get_FTP(self):
         """Return the ftplib.FTP instance for this object."""
@@ -154,15 +156,6 @@
         """
         return True
 
-    def clone(self, offset=None):
-        """Return a new FtpTransport with root at self.base + offset.
-        """
-        mutter("FTP clone")
-        if offset is None:
-            return FtpTransport(self.base, self._FTP_instance)
-        else:
-            return FtpTransport(self.abspath(offset), self._FTP_instance)
-
     def _remote_path(self, relpath):
         # XXX: It seems that ftplib does not handle Unicode paths
         # at the same time, medusa won't handle utf8 paths So if

=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py	2007-06-01 10:02:05 +0000
+++ b/bzrlib/transport/http/__init__.py	2007-06-01 20:26:46 +0000
@@ -152,9 +152,10 @@
         else:
             self._range_hint = 'multi'
 
-    def _unqualified_abspath(self, relpath):
+    def _remote_path(self, relpath):
         """Produce absolute path, adjusting protocol if needed"""
-        path = self._remote_path(relpath)
+        relative = urlutils.unescape(relpath).encode('utf-8')
+        path = self._combine_paths(self._path, relative)
         return self._unsplit_url(self._unqualified_scheme,
                                  self._user, self._password,
                                  self._host, self._port,

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py	2007-06-01 10:02:05 +0000
+++ b/bzrlib/transport/http/_pycurl.py	2007-06-01 20:26:46 +0000
@@ -112,7 +112,7 @@
         # We set NO BODY=0 in _get_full, so it should be safe
         # to re-use the non-range curl object
         curl = self._curl
-        abspath = self._unqualified_abspath(relpath)
+        abspath = self._remote_path(relpath)
         curl.setopt(pycurl.URL, abspath)
         self._set_curl_options(curl)
         curl.setopt(pycurl.HTTPGET, 1)
@@ -161,7 +161,7 @@
                  data: file that will be filled with the body
                  header: file that will be filled with the headers
         """
-        abspath = self._unqualified_abspath(relpath)
+        abspath = self._remote_path(relpath)
         curl.setopt(pycurl.URL, abspath)
         self._set_curl_options(curl)
 

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2007-06-01 10:02:05 +0000
+++ b/bzrlib/transport/http/_urllib.py	2007-06-01 20:26:46 +0000
@@ -64,7 +64,7 @@
             self._connection = None
             self._opener = self._opener_class()
 
-            authuri = extract_authentication_uri(self._unqualified_abspath(self._path))
+            authuri = extract_authentication_uri(self._remote_path(self._path))
             self._auth = {'user': user, 'password': password,
                           'authuri': authuri}
             if user and password is not None: # '' is a valid password
@@ -116,7 +116,7 @@
     def _get(self, relpath, ranges, tail_amount=0):
         """See HttpTransport._get"""
 
-        abspath = self._unqualified_abspath(relpath)
+        abspath = self._remote_path(relpath)
         headers = {}
         if ranges or tail_amount:
             range_header = self.attempted_range_header(ranges, tail_amount)
@@ -138,7 +138,7 @@
         return code, data
 
     def _post(self, body_bytes):
-        abspath = self._unqualified_abspath('.bzr/smart')
+        abspath = self._remote_path('.bzr/smart')
         response = self._perform(Request('POST', abspath, body_bytes))
         code = response.code
         data = handle_response(abspath, code, response.headers, response)
@@ -156,7 +156,7 @@
 
         Performs the request and leaves callers handle the results.
         """
-        abspath = self._unqualified_abspath(relpath)
+        abspath = self._remote_path(relpath)
         request = Request('HEAD', abspath)
         response = self._perform(request)
 

=== modified file 'bzrlib/transport/sftp.py'
--- a/bzrlib/transport/sftp.py	2007-06-01 08:02:26 +0000
+++ b/bzrlib/transport/sftp.py	2007-06-01 20:26:46 +0000
@@ -146,6 +146,13 @@
 
     def _urldecode_abspath(self, abspath):
         abspath = super(SFTPUrlHandling, self)._urldecode_abspath(abspath)
+        if abspath.startswith('/~/'):
+            abspath = abspath[3:]
+        elif abspath == '/~':
+            abspath = ''
+        return abspath
+
+    def _combine_paths_respecting_home_dir(self, relpath):
         # the initial slash should be removed from the path, and treated
         # as a homedir relative path (the path begins with a double slash
         # if it is absolute).
@@ -153,28 +160,33 @@
         # RBC 20060118 we are not using this as its too user hostile. instead
         # we are following lftp and using /~/foo to mean '~/foo'.
 
-        # handle homedir paths
-        if abspath.startswith('/~/'):
-            abspath = abspath[3:]
-        elif abspath == '/~':
-            abspath = ''
+        if not self._path.startswith('/'):
+            abspath = self._combine_paths('/~/' + self._path, relpath)
+            if abspath.startswith('/~/'):
+                abspath = abspath[3:]
+            elif abspath == '/~':
+                abspath = ''
+        else:
+            abspath = self._combine_paths(self._path, relpath)
+
         return abspath
 
+    def abspath(self, relpath):
+        """Return the full url to the given relative path respecting home dir"""
+        relative = urlutils.unescape(relpath).encode('utf-8')
+        path = self._combine_paths_respecting_home_dir(relative)
+        return self._unsplit_url(self._scheme, self._user, self._password,
+                                 self._host, self._port,
+                                 self._urlencode_abspath(path))
+
+
     def _remote_path(self, relpath):
         """Return the path to be passed along the sftp protocol for relpath.
         
         :param relpath: is a urlencoded string.
         """
         relative = urlutils.unescape(relpath).encode('utf-8')
-        if not self._path.startswith('/'):
-            # handle homedir paths
-            remote_path = self._combine_paths('/~/' + self._path, relative)
-            if remote_path.startswith('/~/'):
-                remote_path = remote_path[3:]
-            elif remote_path == '/~':
-                remote_path = ''
-        else:
-            remote_path = self._combine_paths(self._path, relative)
+        remote_path = self._combine_paths_respecting_home_dir(relative)
         return remote_path
 
 class SFTPTransport(SFTPUrlHandling):
@@ -198,14 +210,14 @@
     # up the request itself, rather than us having to worry about it
     _max_request_size = 32768
 
-    def __init__(self, base, clone_from=None):
+    def __init__(self, base, from_transport=None):
         assert base.startswith('sftp://')
-        super(SFTPTransport, self).__init__(base)
-        if clone_from is None:
+        super(SFTPTransport, self).__init__(base, from_transport)
+        if from_transport is None:
             self._sftp_connect()
         else:
             # use the same ssh connection, etc
-            self._sftp = clone_from._sftp
+            self._sftp = from_transport._sftp
         # super saves 'self.base'
     
     def should_cache(self):



More information about the bazaar-commits mailing list