Rev 3882: Cleanup. in lp:~vila/bzr/303959-redirection

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu Dec 4 15:53:43 GMT 2008


At lp:~vila/bzr/303959-redirection

------------------------------------------------------------
revno: 3882
revision-id: v.ladeuil+lp at free.fr-20081204155340-atj2k9w0cbcuob2k
parent: v.ladeuil+lp at free.fr-20081204115030-4dytul8qv82viscm
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 303959-redirection
timestamp: Thu 2008-12-04 16:53:40 +0100
message:
  Cleanup.
  
  * bzrlib/transport/http/_urllib.py:
  (HttpTransport_urllib._perform): Update RedirectRequested creation.
  
  * bzrlib/transport/http/_pycurl.py:
  (PyCurlTransport._curl_perform): Update RedirectRequested creation.
  
  * bzrlib/tests/blackbox/test_push.py:
  (RedirectingMemoryTransport.mkdir): RedirectRequested wants urls
  not paths. Be more precise with the paths used too.
  (TestPushRedirect.test_push_redirects_on_mkdir,
  TestPushRedirect.test_push_gracefully_handles_too_many_redirects):
  Use '-d' instead of chdir().
  
  * bzrlib/push.py:
  (_show_push_branch.redirected): Updated and note() added (why
  wasn't it there in the first place ?).
  
  * bzrlib/errors.py:
  (RedirectRequested): Simplified now that the redirection stuff is
  handled by the transport itself.
modified:
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/push.py                 push.py-20080606021927-5fe39050e8xne9un-1
  bzrlib/tests/blackbox/test_push.py test_push.py-20060329002750-929af230d5d22663
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
  bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
-------------- next part --------------
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2008-11-27 07:25:52 +0000
+++ b/bzrlib/errors.py	2008-12-04 15:53:40 +0000
@@ -1661,49 +1661,15 @@
 
     _fmt = '%(source)s is%(permanently)s redirected to %(target)s'
 
-    def __init__(self, source, target, is_permanent=False, qual_proto=None):
+    def __init__(self, source, target, is_permanent=False):
         self.source = source
         self.target = target
         if is_permanent:
             self.permanently = ' permanently'
         else:
             self.permanently = ''
-        self._qualified_proto = qual_proto
         TransportError.__init__(self)
 
-    def _requalify_url(self, url):
-        """Restore the qualified proto in front of the url"""
-        # When this exception is raised, source and target are in
-        # user readable format. But some transports may use a
-        # different proto (http+urllib:// will present http:// to
-        # the user. If a qualified proto is specified, the code
-        # trapping the exception can get the qualified urls to
-        # properly handle the redirection themself (creating a
-        # new transport object from the target url for example).
-        # But checking that the scheme of the original and
-        # redirected urls are the same can be tricky. (see the
-        # FIXME in BzrDir.open_from_transport for the unique use
-        # case so far).
-        if self._qualified_proto is None:
-            return url
-
-        # The TODO related to NotBranchError mention that doing
-        # that kind of manipulation on the urls may not be the
-        # exception object job. On the other hand, this object is
-        # the interface between the code and the user so
-        # presenting the urls in different ways is indeed its
-        # job...
-        import urlparse
-        proto, netloc, path, query, fragment = urlparse.urlsplit(url)
-        return urlparse.urlunsplit((self._qualified_proto, netloc, path,
-                                   query, fragment))
-
-    def get_source_url(self):
-        return self._requalify_url(self.source)
-
-    def get_target_url(self):
-        return self._requalify_url(self.target)
-
 
 class TooManyRedirections(TransportError):
 

=== modified file 'bzrlib/push.py'
--- a/bzrlib/push.py	2008-08-28 19:35:29 +0000
+++ b/bzrlib/push.py	2008-12-04 15:53:40 +0000
@@ -74,8 +74,9 @@
             transport.mkdir('.')
             return transport
 
-        def redirected(redirected_transport, e, redirection_notice):
-            return transport.get_transport(e.get_target_url())
+        def redirected(transport, e, redirection_notice):
+            note(redirection_notice)
+            return transport._redirected_to(e)
 
         try:
             to_transport = transport.do_catching_redirections(

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- a/bzrlib/tests/blackbox/test_push.py	2008-09-25 22:25:09 +0000
+++ b/bzrlib/tests/blackbox/test_push.py	2008-12-04 15:53:40 +0000
@@ -22,6 +22,7 @@
 
 from bzrlib import (
     errors,
+    transport,
     urlutils,
     )
 from bzrlib.branch import Branch
@@ -30,7 +31,6 @@
 from bzrlib.repofmt.knitrepo import RepositoryFormatKnit1
 from bzrlib.tests.blackbox import ExternalBase
 from bzrlib.tests.http_server import HttpServer
-from bzrlib.transport import register_transport, unregister_transport
 from bzrlib.transport.memory import MemoryServer, MemoryTransport
 from bzrlib.uncommit import uncommit
 from bzrlib.urlutils import local_path_from_url
@@ -353,17 +353,24 @@
 
 class RedirectingMemoryTransport(MemoryTransport):
 
-    def mkdir(self, path, mode=None):
-        path = self.abspath(path)[len(self._scheme):]
-        if path == '/source':
-            raise errors.RedirectRequested(
-                path, self._scheme + '/target', is_permanent=True)
-        elif path == '/infinite-loop':
-            raise errors.RedirectRequested(
-                path, self._scheme + '/infinite-loop', is_permanent=True)
+    def mkdir(self, relpath, mode=None):
+        from bzrlib.trace import mutter
+        mutter('cwd: %r, rel: %r, abs: %r' % (self._cwd, relpath, abspath))
+        if self._cwd == '/source/':
+            raise errors.RedirectRequested(self.abspath(relpath),
+                                           self.abspath('../target'),
+                                           is_permanent=True)
+        elif self._cwd == '/infinite-loop/':
+            raise errors.RedirectRequested(self.abspath(relpath),
+                                           self.abspath('../infinite-loop'),
+                                           is_permanent=True)
         else:
             return super(RedirectingMemoryTransport, self).mkdir(
-                path, mode)
+                relpath, mode)
+
+    def _redirected_to(self, exception):
+        # We do accept redirections
+        return transport.get_transport(exception.target)
 
 
 class RedirectingMemoryServer(MemoryServer):
@@ -373,7 +380,7 @@
         self._files = {}
         self._locks = {}
         self._scheme = 'redirecting-memory+%s:///' % id(self)
-        register_transport(self._scheme, self._memory_factory)
+        transport.register_transport(self._scheme, self._memory_factory)
 
     def _memory_factory(self, url):
         result = RedirectingMemoryTransport(url)
@@ -383,7 +390,7 @@
         return result
 
     def tearDown(self):
-        unregister_transport(self._scheme, self._memory_factory)
+        transport.unregister_transport(self._scheme, self._memory_factory)
 
 
 class TestPushRedirect(ExternalBase):
@@ -406,10 +413,8 @@
         This is added primarily to handle lp:/ URI support, so that users can
         push to new branches by specifying lp:/ URIs.
         """
-        os.chdir('tree')
         destination_url = self.memory_server.get_url() + 'source'
-        self.run_bzr('push %s' % destination_url)
-        os.chdir('..')
+        self.run_bzr(['push', '-d', 'tree', destination_url])
 
         local_revision = Branch.open('tree').last_revision()
         remote_revision = Branch.open(
@@ -420,11 +425,9 @@
         """Push fails gracefully if the mkdir generates a large number of
         redirects.
         """
-        os.chdir('tree')
         destination_url = self.memory_server.get_url() + 'infinite-loop'
         out, err = self.run_bzr_error(
             ['Too many redirections trying to make %s\\.\n'
              % re.escape(destination_url)],
-            'push %s' % destination_url, retcode=3)
-        os.chdir('..')
+            ['push', '-d', 'tree', destination_url], retcode=3)
         self.assertEqual('', out)

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2008-12-04 11:50:30 +0000
+++ b/bzrlib/tests/test_http.py	2008-12-04 15:53:40 +0000
@@ -1749,8 +1749,7 @@
     def test_redirected_to_subdir(self):
         t = self._transport('http://www.example.com/foo')
         e = errors.RedirectRequested('http://www.example.com/foo',
-                                     'http://www.example.com/foo/subdir',
-                                     self._qualified_prefix)
+                                     'http://www.example.com/foo/subdir')
         r = t._redirected_to(e)
         self.assertIsInstance(r, type(t))
         # Both transports share the some connection
@@ -1759,8 +1758,7 @@
     def test_redirected_to_self_with_slash(self):
         t = self._transport('http://www.example.com/foo')
         e = errors.RedirectRequested('http://www.example.com/foo',
-                                     'http://www.example.com/foo/',
-                                     self._qualified_prefix)
+                                     'http://www.example.com/foo/')
         r = t._redirected_to(e)
         self.assertIsInstance(r, type(t))
         # Both transports share the some connection (one can argue that we
@@ -1779,24 +1777,21 @@
     def test_redirected_to_same_host_sibling_protocol(self):
         t = self._transport('http://www.example.com/foo')
         e = errors.RedirectRequested('http://www.example.com/foo',
-                                     'https://www.example.com/foo',
-                                     self._qualified_prefix)
+                                     'https://www.example.com/foo')
         r = t._redirected_to(e)
         self.assertIsInstance(r, type(t))
 
     def test_redirected_to_same_host_different_protocol(self):
         t = self._transport('http://www.example.com/foo')
         e = errors.RedirectRequested('http://www.example.com/foo',
-                                     'ftp://www.example.com/foo',
-                                     self._qualified_prefix)
+                                     'ftp://www.example.com/foo')
         r = t._redirected_to(e)
         self.assertNotEquals(type(r), type(t))
 
     def test_redirected_to_different_host_same_user(self):
         t = self._transport('http://joe@www.example.com/foo')
         e = errors.RedirectRequested('http://www.example.com/foo',
-                                     'https://foo.example.com/foo',
-                                     self._qualified_prefix)
+                                     'https://foo.example.com/foo')
         r = t._redirected_to(e)
         self.assertIsInstance(r, type(t))
         self.assertEquals(t._user, r._user)

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py	2008-12-04 11:25:56 +0000
+++ b/bzrlib/transport/http/_pycurl.py	2008-12-04 15:53:40 +0000
@@ -367,8 +367,7 @@
             redirected_to = msg.getheader('location')
             raise errors.RedirectRequested(url,
                                            redirected_to,
-                                           is_permanent=(code == 301),
-                                           qual_proto=self._scheme)
+                                           is_permanent=(code == 301))
 
 
 def get_test_permutations():

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2008-12-04 11:25:56 +0000
+++ b/bzrlib/transport/http/_urllib.py	2008-12-04 15:53:40 +0000
@@ -90,8 +90,7 @@
                 and code in (301, 302, 303, 307):
             raise errors.RedirectRequested(request.get_full_url(),
                                            request.redirected_to,
-                                           is_permanent=(code == 301),
-                                           qual_proto=self._scheme)
+                                           is_permanent=(code == 301))
 
         if request.redirected_to is not None:
             trace.mutter('redirected from: %s to: %s' % (request.get_full_url(),



More information about the bazaar-commits mailing list