← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/explicit-proxy-mantis into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/explicit-proxy-mantis into lp:launchpad with lp:~cjwatson/launchpad/explicit-proxy-sourceforge as a prerequisite.

Commit message:
Convert the Mantis external bug tracker to urlfetch.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/explicit-proxy-mantis/+merge/348431

The login arrangements require some care, but fortunately requests hooks are powerful enough for this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/explicit-proxy-mantis into lp:launchpad.
=== modified file 'lib/lp/bugs/doc/externalbugtracker-mantis-csv.txt'
--- lib/lp/bugs/doc/externalbugtracker-mantis-csv.txt	2012-04-12 11:38:44 +0000
+++ lib/lp/bugs/doc/externalbugtracker-mantis-csv.txt	2018-06-23 00:29:08 +0000
@@ -67,8 +67,8 @@
 calls to verify here. We set its batch_query_threshold to 0 so as to
 force it to use the CSV import code:
 
-    >>> example_ext_bug_tracker = TestMantis(example_bug_tracker.baseurl)
-    >>> example_ext_bug_tracker.batch_query_threshold = 0
+    >>> mantis = TestMantis(example_bug_tracker.baseurl)
+    >>> mantis.batch_query_threshold = 0
 
 Collect the Example.com watches:
 
@@ -84,8 +84,9 @@
     >>> from lp.bugs.scripts.checkwatches import CheckwatchesMaster
     >>> bug_watch_updater = CheckwatchesMaster(
     ...     transaction, logger=FakeLogger())
-    >>> bug_watch_updater.updateBugWatches(
-    ...     example_ext_bug_tracker, example_bug_tracker.watches)
+    >>> with mantis.responses():
+    ...     bug_watch_updater.updateBugWatches(
+    ...         mantis, example_bug_tracker.watches)
     INFO Updating 1 watches for 1 bugs on http://bugs.some.where
 
     >>> for bug_watch in example_bug_tracker.watches:
@@ -117,7 +118,7 @@
     ...         bug=example_bug, owner=sample_person,
     ...         bugtracker=example_bug_tracker,
     ...         remotebug=str(remote_bug_id))
-    ...     example_ext_bug_tracker.bugs[remote_bug_id] = remote_bug
+    ...     mantis.bugs[remote_bug_id] = remote_bug
     ...     expected_remote_statuses[remote_bug_id] = (
     ...         "%s: %s" % (remote_bug['status'], remote_bug['resolution']))
 
@@ -125,14 +126,14 @@
 updateBugWatches() issues only one request to update all watches:
 
     >>> from lp.services.propertycache import get_property_cache
-    >>> del get_property_cache(example_ext_bug_tracker).csv_data
+    >>> del get_property_cache(mantis).csv_data
 
-    >>> example_ext_bug_tracker.trace_calls = True
-    >>> bug_watch_updater.updateBugWatches(
-    ...     example_ext_bug_tracker, example_bug_tracker.watches)
+    >>> with mantis.responses(trace_calls=True):
+    ...     bug_watch_updater.updateBugWatches(
+    ...         mantis, example_bug_tracker.watches)
     INFO Updating 6 watches for 6 bugs on http://bugs.some.where
-    CALLED _postPage('view_all_set.php?f=3', ...)
-    CALLED _getPage('csv_export.php')
+    POST http://bugs.some.where/view_all_set.php?f=3
+    GET http://bugs.some.where/csv_export.php
 
     >>> remote_statuses = dict(
     ...     (int(bug_watch.remotebug), bug_watch.remotestatus)
@@ -164,8 +165,6 @@
      * Expected << assigned: open >>
      *      Got << assigned: open >>
 
-    >>> example_ext_bug_tracker.trace_calls = False
-
 updateBugWatches() updates the lastchecked attribute on the watches, so
 now no bug watches are in need of updating:
 
@@ -183,8 +182,7 @@
     >>> now = datetime.now(pytz.timezone('UTC'))
     >>> bug_watch.lastchanged = now - timedelta(weeks=2)
     >>> old_last_changed = bug_watch.lastchanged
-    >>> bug_watch_updater.updateBugWatches(
-    ...     example_ext_bug_tracker, [bug_watch])
+    >>> bug_watch_updater.updateBugWatches(mantis, [bug_watch])
     INFO Updating 1 watches for 1 bugs on http://bugs.some.where
 
     >>> bug_watch.lastchanged == old_last_changed

=== modified file 'lib/lp/bugs/doc/externalbugtracker-mantis-logging-in.txt'
--- lib/lp/bugs/doc/externalbugtracker-mantis-logging-in.txt	2011-01-20 23:10:56 +0000
+++ lib/lp/bugs/doc/externalbugtracker-mantis-logging-in.txt	2018-06-23 00:29:08 +0000
@@ -3,40 +3,49 @@
 
 Mantis is... special. In a headless/batch environment we must do a
 little dance in order to log in. Thankfully, we can encapsulate this
-neatly in a urllib2-style handler.
-
-
-MantisLoginHandler
-==================
-
-    >>> from lp.bugs.externalbugtracker import (
-    ...     MantisLoginHandler)
-    >>> handler = MantisLoginHandler()
+neatly in a requests hook.
+
+
+mantis_login_hook
+=================
+
+    >>> from requests import Response
+    >>> from lp.bugs.externalbugtracker.mantis import mantis_login_hook
+
+    >>> def run_hook(url, status_code=302):
+    ...     response = Response()
+    ...     response.status_code = status_code
+    ...     response.headers['Location'] = url
+    ...     response = mantis_login_hook(response)
+    ...     print(response.headers['Location'])
 
 It has one simple responsibility, which is to intercept redirections
 to the login page, at which point it rewrites the URL to go straight
 to the login form handler with a default user name and password.
 
-MantisLoginHandler.redirect_request is a standard urllib2-style
-method, whose only job is to call MantisLoginHandler.rewrite_url
-before calling the superclass. We're only interested in rewrite_url.
-
-Normally rewrite_url makes no modifications to the URL:
-
-    >>> handler.rewrite_url('https://mantis.example.com/')
-    'https://mantis.example.com/'
-
-    >>> handler.rewrite_url('http://mantis.example.com/view.php?id=123')
-    'http://mantis.example.com/view.php?id=123'
-
-When Mantis redirects us to the login page, rewrite_url comes into
+Normally the hook makes no modifications to the URL:
+
+    >>> run_hook('https://mantis.example.com/')
+    https://mantis.example.com/
+
+    >>> run_hook('http://mantis.example.com/view.php?id=123')
+    http://mantis.example.com/view.php?id=123
+
+The hook doesn't touch any non-redirect responses.
+
+    >>> run_hook(
+    ...     'http://mantis.example.com/login_page.php'
+    ...     '?return=%2Fview.php%3Fid%3D3301', status_code=200)
+    http://.../login_page.php?return=%2Fview.php%3Fid%3D3301
+
+When Mantis redirects us to the login page, the hook comes into
 play. Note how Mantis adds a "return" query parameter: if we log in
 successfully, Mantis will redirect us to the page this specifies.
 
-    >>> handler.rewrite_url(
+    >>> run_hook(
     ...     'http://mantis.example.com/login_page.php'
     ...     '?return=%2Fview.php%3Fid%3D3301')
-    '.../login.php?username=guest&password=guest&return=...'
+    http://.../login.php?username=guest&password=guest&return=...
 
 If Mantis does not specify a "return" query parameter an error will be
 raised.
@@ -46,8 +55,7 @@
 when Mantis redirects back to the login page with an error it forgets
 the "return" parameter.
 
-    >>> handler.rewrite_url(
-    ...     'http://mantis.example.com/login_page.php')
+    >>> run_hook('http://mantis.example.com/login_page.php')
     Traceback (most recent call last):
       ...
     BugTrackerConnectError: http://mantis.example.com/login_page.php:

=== modified file 'lib/lp/bugs/doc/externalbugtracker-mantis.txt'
--- lib/lp/bugs/doc/externalbugtracker-mantis.txt	2012-12-26 01:32:19 +0000
+++ lib/lp/bugs/doc/externalbugtracker-mantis.txt	2018-06-23 00:29:08 +0000
@@ -68,7 +68,7 @@
 We use a specially hacked Mantis instance that doesn't do network
 calls to verify here:
 
-    >>> example_ext_bug_tracker = TestMantis(example_bug_tracker.baseurl)
+    >>> mantis = TestMantis(example_bug_tracker.baseurl)
 
 Collect the Example.com watches:
 
@@ -86,8 +86,9 @@
     >>> txn = LaunchpadZopelessLayer.txn
     >>> bug_watch_updater = CheckwatchesMaster(
     ...     txn, logger=FakeLogger())
-    >>> bug_watch_updater.updateBugWatches(
-    ...     example_ext_bug_tracker, example_bug_tracker.watches)
+    >>> with mantis.responses(post=False):
+    ...     bug_watch_updater.updateBugWatches(
+    ...         mantis, example_bug_tracker.watches)
     INFO Updating 1 watches for 1 bugs on http://bugs.some.where
 
     >>> for bug_watch in example_bug_tracker.watches:
@@ -133,19 +134,18 @@
     >>> from operator import attrgetter
     >>> getid = attrgetter('id')
 
-    >>> example_ext_bug_tracker.trace_calls = True
-    >>> bug_watch_updater.updateBugWatches(
-    ...     example_ext_bug_tracker,
-    ...     sorted(example_bug_tracker.watches, key=getid))
+    >>> with mantis.responses(trace_calls=True, post=False):
+    ...     bug_watch_updater.updateBugWatches(
+    ...         mantis, sorted(example_bug_tracker.watches, key=getid))
     INFO Updating 7 watches for 6 bugs on http://bugs.some.where
-    CALLED _getPage(u'view.php?id=1550')
-    CALLED _getPage(u'view.php?id=1679')
-    CALLED _getPage(u'view.php?id=1730')
-    CALLED _getPage(u'view.php?id=1738')
-    CALLED _getPage(u'view.php?id=1748')
-    CALLED _getPage(u'view.php?id=1798')
     INFO Didn't find bug u'1798' on http://bugs.some.where
     (local bugs: 10).
+    GET http://bugs.some.where/view.php?id=1550
+    GET http://bugs.some.where/view.php?id=1679
+    GET http://bugs.some.where/view.php?id=1730
+    GET http://bugs.some.where/view.php?id=1738
+    GET http://bugs.some.where/view.php?id=1748
+    GET http://bugs.some.where/view.php?id=1798
 
     >>> remote_statuses = dict(
     ...     (int(bug_watch.remotebug), bug_watch.remotestatus)
@@ -177,8 +177,6 @@
      * Expected << None >>
      *      Got << None >>
 
-    >>> example_ext_bug_tracker.trace_calls = False
-
 updateBugWatches() updates the lastchecked attribute on the watches, so
 now no bug watches are in need of updating:
 
@@ -197,8 +195,8 @@
     >>> bug_watch.lastchanged = now - timedelta(weeks=2)
     >>> bug_watch.lastchecked = bug_watch.lastchanged
     >>> old_last_changed = bug_watch.lastchanged
-    >>> bug_watch_updater.updateBugWatches(
-    ...     example_ext_bug_tracker, [bug_watch])
+    >>> with mantis.responses(post=False):
+    ...     bug_watch_updater.updateBugWatches(mantis, [bug_watch])
     INFO Updating 1 watches for 1 bugs on http://bugs.some.where
 
     >>> bug_watch.lastchanged == old_last_changed

=== modified file 'lib/lp/bugs/externalbugtracker/__init__.py'
--- lib/lp/bugs/externalbugtracker/__init__.py	2018-06-23 00:29:08 +0000
+++ lib/lp/bugs/externalbugtracker/__init__.py	2018-06-23 00:29:08 +0000
@@ -19,7 +19,6 @@
     'InvalidBugId',
     'LookupTree',
     'Mantis',
-    'MantisLoginHandler',
     'PrivateRemoteBug',
     'RequestTracker',
     'Roundup',
@@ -56,10 +55,7 @@
     DebBugsDatabaseNotFound,
     )
 from lp.bugs.externalbugtracker.github import GitHub
-from lp.bugs.externalbugtracker.mantis import (
-    Mantis,
-    MantisLoginHandler,
-    )
+from lp.bugs.externalbugtracker.mantis import Mantis
 from lp.bugs.externalbugtracker.roundup import Roundup
 from lp.bugs.externalbugtracker.rt import RequestTracker
 from lp.bugs.externalbugtracker.sourceforge import SourceForge

=== modified file 'lib/lp/bugs/externalbugtracker/mantis.py'
--- lib/lp/bugs/externalbugtracker/mantis.py	2017-10-21 18:14:14 +0000
+++ lib/lp/bugs/externalbugtracker/mantis.py	2018-06-23 00:29:08 +0000
@@ -1,25 +1,30 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Mantis ExternalBugTracker utility."""
 
 __metaclass__ = type
-__all__ = ['Mantis', 'MantisLoginHandler']
+__all__ = [
+    'Mantis',
+    'mantis_login_hook',
+    ]
 
-import cgi
 import csv
 import logging
-import urllib
-import urllib2
-from urlparse import urlunparse
 
 from BeautifulSoup import Comment
+from requests.cookies import RequestsCookieJar
+from six.moves.urllib_parse import (
+    parse_qsl,
+    urlencode,
+    urlunparse,
+    )
 
 from lp.bugs.externalbugtracker import (
     BugNotFound,
     BugTrackerConnectError,
     BugWatchUpdateError,
-    ExternalBugTracker,
+    ExternalBugTrackerRequests,
     InvalidBugId,
     LookupTree,
     UnknownRemoteStatusError,
@@ -38,9 +43,8 @@
 from lp.services.webapp.url import urlparse
 
 
-class MantisLoginHandler(urllib2.HTTPRedirectHandler):
-    """Handler for urllib2.build_opener to automatically log-in
-    to Mantis anonymously if needed.
+def mantis_login_hook(response, *args, **kwargs):
+    """requests hook to automatically log into Mantis anonymously if needed.
 
     The ALSA bug tracker is the only tested Mantis installation that
     actually needs this. For ALSA bugs, the dance is like so:
@@ -60,42 +64,41 @@
       4. Mantis accepts our credentials then redirects us to the bug
          view page via a cookie test page (login_cookie_test.php)
     """
-
-    def rewrite_url(self, url):
-        scheme, host, path, params, query, fragment = urlparse(url)
-
-        # If we can, skip the login page and submit credentials
-        # directly. The query should contain a 'return' parameter
-        # which, if our credentials are accepted, means we'll be
-        # redirected back from whence we came. In other words, we'll
-        # end up back at the bug page we first requested.
-        login_page = '/login_page.php'
-        if path.endswith(login_page):
-            path = path[:-len(login_page)] + '/login.php'
-            query = cgi.parse_qs(query, True)
-            query['username'] = query['password'] = ['guest']
-            if 'return' not in query:
-                raise BugTrackerConnectError(
-                    url, ("Mantis redirected us to the login page "
-                          "but did not set a return path."))
-
-            query = urllib.urlencode(query, True)
-            url = urlunparse(
-                (scheme, host, path, params, query, fragment))
-
-        # Previous versions of the Mantis external bug tracker fetched
-        # login_anon.php in addition to the login.php method above, but none
-        # of the Mantis installations tested actually needed this. For
-        # example, the ALSA bugtracker actually issues an error "Your account
-        # may be disabled" when accessing this page. For now it's better to
-        # *not* try this page because we may end up annoying admins with
-        # spurious login attempts.
-
-        return url
-
-    def redirect_request(self, request, fp, code, msg, hdrs, new_url):
-        return urllib2.HTTPRedirectHandler.redirect_request(
-            self, request, fp, code, msg, hdrs, self.rewrite_url(new_url))
+    if response.status_code not in (301, 302, 303, 307):
+        return response
+    if 'Location' not in response.headers:
+        return response
+
+    url = response.headers['Location']
+    scheme, host, path, params, query, fragment = urlparse(url)
+
+    # If we can, skip the login page and submit credentials directly.  The
+    # query should contain a 'return' parameter which, if our credentials
+    # are accepted, means we'll be redirected back whence we came.  In other
+    # words, we'll end up back at the bug page we first requested.
+    login_page = '/login_page.php'
+    if path.endswith(login_page):
+        path = path[:-len(login_page)] + '/login.php'
+        query_list = [('username', 'guest'), ('password', 'guest')]
+        query_list.extend(parse_qsl(query, True))
+        if not any(name == 'return' for name, _ in query_list):
+            raise BugTrackerConnectError(
+                url, ("Mantis redirected us to the login page "
+                      "but did not set a return path."))
+
+        query = urlencode(query_list, True)
+        url = urlunparse((scheme, host, path, params, query, fragment))
+
+    # Previous versions of the Mantis external bug tracker fetched
+    # login_anon.php in addition to the login.php method above, but none of
+    # the Mantis installations tested actually needed this.  For example,
+    # the ALSA bugtracker actually issues an error "Your account may be
+    # disabled" when accessing this page.  For now it's better to *not* try
+    # this page because we may end up annoying admins with spurious login
+    # attempts.
+
+    response.headers['Location'] = url
+    return response
 
 
 class MantisBugBatchParser:
@@ -162,7 +165,7 @@
             raise UnparsableBugData("Exception parsing CSV file: %s." % error)
 
 
-class Mantis(ExternalBugTracker):
+class Mantis(ExternalBugTrackerRequests):
     """An `ExternalBugTracker` for dealing with Mantis instances.
 
     For a list of tested Mantis instances and their behaviour when
@@ -173,13 +176,18 @@
 
     def __init__(self, baseurl):
         super(Mantis, self).__init__(baseurl)
-        # Custom cookie aware opener that automatically sends anonymous
-        # credentials to Mantis if (and only if) needed.
-        self._cookie_handler = urllib2.HTTPCookieProcessor()
-        self.url_opener = urllib2.build_opener(
-            self._cookie_handler, MantisLoginHandler())
+        self._cookie_jar = RequestsCookieJar()
         self._logger = logging.getLogger()
 
+    def makeRequest(self, method, url, hooks=None, **kwargs):
+        """See `ExternalBugTracker`."""
+        hooks = dict(hooks) if hooks is not None else {}
+        if not isinstance(hooks.setdefault('response', []), list):
+            hooks['response'] = [hooks['response']]
+        hooks['response'].append(mantis_login_hook)
+        return super(Mantis, self).makeRequest(
+            method, url, cookies=self._cookie_jar, hooks=hooks, **kwargs)
+
     @cachedproperty
     def csv_data(self):
         """Attempt to retrieve a CSV export from the remote server.
@@ -241,13 +249,14 @@
         # MANTIS_VIEW_ALL_COOKIE set in the previous step to specify
         # what's being viewed.
         try:
-            csv_data = self._getPage("csv_export.php")
+            csv_data = self._getPage("csv_export.php").content
         except BugTrackerConnectError as value:
             # Some Mantis installations simply return a 500 error
             # when the csv_export.php page is accessed. Since the
             # bug data may be nevertheless available from ordinary
             # web pages, we simply ignore this error.
-            if value.error.startswith('HTTP Error 500'):
+            if (value.error.response is not None and
+                    value.error.response.status_code == 500):
                 return None
             raise
 
@@ -295,7 +304,7 @@
         # _checkForApplicationError) then we could be much more
         # specific than this.
         bug_page = BeautifulSoup(
-            self._getPage('view.php?id=%s' % bug_id),
+            self._getPage('view.php?id=%s' % bug_id).content,
             convertEntities=BeautifulSoup.HTML_ENTITIES,
             parseOnlyThese=SoupStrainer('table'))
 

=== modified file 'lib/lp/bugs/externalbugtracker/tests/test_mantis.py'
--- lib/lp/bugs/externalbugtracker/tests/test_mantis.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/externalbugtracker/tests/test_mantis.py	2018-06-23 00:29:08 +0000
@@ -1,12 +1,12 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the Mantis BugTracker."""
 
 __metaclass__ = type
 
-import urllib2
-
+import responses
+from six.moves.urllib_parse import urljoin
 from testtools.matchers import (
     Equals,
     Is,
@@ -18,11 +18,7 @@
     MantisBugBatchParser,
     )
 from lp.services.log.logger import BufferLogger
-from lp.testing import (
-    monkey_patch,
-    TestCase,
-    )
-from lp.testing.fakemethod import FakeMethod
+from lp.testing import TestCase
 from lp.testing.layers import ZopelessLayer
 
 
@@ -107,17 +103,16 @@
 
 
 class TestMantisBugTracker(TestCase):
-    """Tests for various methods of the Manits bug tracker."""
+    """Tests for various methods of the Mantis bug tracker."""
 
     layer = ZopelessLayer
 
+    @responses.activate
     def test_csv_data_on_post_404(self):
         # If the 'view_all_set.php' request raises a 404, then the csv_data
         # attribute is None.
         base_url = "http://example.com/";
-
-        fail_404 = urllib2.HTTPError('url', 404, 'Not Found', None, None)
-
-        with monkey_patch(Mantis, urlopen=FakeMethod(failure=fail_404)):
-            bugtracker = Mantis(base_url)
-            self.assertThat(bugtracker.csv_data, Is(None))
+        responses.add(
+            "POST", urljoin(base_url, "view_all_set.php?f=3"), status=404)
+        bugtracker = Mantis(base_url)
+        self.assertThat(bugtracker.csv_data, Is(None))

=== modified file 'lib/lp/bugs/tests/externalbugtracker.py'
--- lib/lp/bugs/tests/externalbugtracker.py	2018-06-23 00:29:08 +0000
+++ lib/lp/bugs/tests/externalbugtracker.py	2018-06-23 00:29:08 +0000
@@ -1177,30 +1177,27 @@
         }
 
 
-class TestMantis(Mantis):
-    """Mantis ExternalSystem for use in tests.
-
-    It overrides _getPage and _postPage, so that access to a real
-    Mantis instance isn't needed.
-    """
-
-    trace_calls = False
-
-    def _getPage(self, page):
-        if self.trace_calls:
-            print "CALLED _getPage(%r)" % (page)
-        if page == "csv_export.php":
-            return read_test_file('mantis_example_bug_export.csv')
-        elif page.startswith('view.php?id='):
-            bug_id = page.split('id=')[-1]
-            return read_test_file('mantis--demo--bug-%s.html' % bug_id)
+class TestMantis(BugTrackerResponsesMixin, Mantis):
+    """Mantis ExternalBugTracker for use in tests."""
+
+    @staticmethod
+    def _getCallback(request):
+        url = urlsplit(request.url)
+        if url.path == '/csv_export.php':
+            body = read_test_file('mantis_example_bug_export.csv')
+        elif url.path == '/view.php':
+            bug_id = parse_qs(url.query)['id'][0]
+            body = read_test_file('mantis--demo--bug-%s.html' % bug_id)
         else:
-            return ''
+            body = ''
+        return 200, {}, body
 
-    def _postPage(self, page, form, repost_on_redirect=False):
-        if self.trace_calls:
-            print "CALLED _postPage(%r, ...)" % (page)
-        return ''
+    def addResponses(self, requests_mock, get=True, post=True):
+        if get:
+            requests_mock.add_callback(
+                'GET', re.compile(re.escape(self.baseurl)), self._getCallback)
+        if post:
+            requests_mock.add('POST', re.compile(re.escape(self.baseurl)))
 
 
 class TestTrac(Trac):

=== modified file 'lib/lp/bugs/tests/test_bugtracker.py'
--- lib/lp/bugs/tests/test_bugtracker.py	2018-01-02 10:54:31 +0000
+++ lib/lp/bugs/tests/test_bugtracker.py	2018-06-23 00:29:08 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -13,13 +13,16 @@
     NORMALIZE_WHITESPACE,
     )
 import unittest
-from urllib2 import (
-    HTTPError,
-    Request,
-    )
 
 from lazr.lifecycle.snapshot import Snapshot
 from pytz import utc
+import responses
+from six.moves.urllib_parse import urlencode
+from testtools.matchers import (
+    Equals,
+    MatchesListwise,
+    MatchesStructure,
+    )
 import transaction
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
@@ -29,7 +32,6 @@
 from lp.bugs.externalbugtracker import (
     BugTrackerConnectError,
     Mantis,
-    MantisLoginHandler,
     )
 from lp.bugs.interfaces.bugtracker import (
     BugTrackerType,
@@ -40,7 +42,6 @@
     make_bugtracker_name,
     make_bugtracker_title,
     )
-from lp.bugs.tests.externalbugtracker import UrlLib2TransportTestHandler
 from lp.registry.interfaces.person import IPersonSet
 from lp.testing import (
     login,
@@ -286,91 +287,79 @@
         # checkwatches isolation protection code.
         transaction.commit()
 
+    @responses.activate
     def test_mantis_login_redirects(self):
         # The Mantis bug tracker needs a special HTTP redirect handler
         # in order to login in. Ensure that redirects to the page with
         # the login form are indeed changed to redirects the form submit
         # URL.
-        handler = MantisLoginHandler()
-        request = Request('http://mantis.example.com/some/path')
-        # Let's pretend that Mantis sent a redirect request to the
-        # login page.
-        new_request = handler.redirect_request(
-            request, None, 302, None, None,
-            'http://mantis.example.com/login_page.php'
-            '?return=%2Fview.php%3Fid%3D3301')
-        self.assertEqual(
+        location = '/login_page.php?' + urlencode({'return': '/some/page'})
+        responses.add(
+            'GET', 'http://mantis.example.com/some/page',
+            status=302, headers={'Location': location})
+        responses.add(
+            'GET',
             'http://mantis.example.com/login.php?'
-            'username=guest&password=guest&return=%2Fview.php%3Fid%3D3301',
-            new_request.get_full_url())
-
-    def test_mantis_login_redirect_handler_is_used(self):
-        # Ensure that the special Mantis login handler is used
-        # by the Mantis tracker
+            'username=guest&password=guest&return=%2Fsome%2Fpage',
+            match_querystring=True, status=200, body='sentinel')
         tracker = Mantis('http://mantis.example.com')
-        test_handler = UrlLib2TransportTestHandler()
-        test_handler.setRedirect('http://mantis.example.com/login_page.php'
-            '?return=%2Fsome%2Fpage')
-        opener = tracker.url_opener
-        opener.add_handler(test_handler)
-        opener.open('http://mantis.example.com/some/page')
-        # We should now have two entries in the test handler's list
-        # of visited URLs: The original URL we wanted to visit and the
-        # URL changed by the MantisLoginHandler.
+        response = tracker.makeRequest(
+            'GET', 'http://mantis.example.com/some/page')
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(b'sentinel', response.content)
+        # The request visited the original URL followed by the URL rewritten
+        # by mantis_login_hook.
+        self.assertThat(response.history, MatchesListwise([
+            MatchesStructure(
+                status_code=Equals(302),
+                request=MatchesStructure.byEquality(
+                    url='http://mantis.example.com/some/page')),
+            ]))
         self.assertEqual(
-            ['http://mantis.example.com/some/page',
-             'http://mantis.example.com/login.php?'
-             'username=guest&password=guest&return=%2Fsome%2Fpage'],
-            test_handler.accessed_urls)
+            'http://mantis.example.com/login.php?'
+            'username=guest&password=guest&return=%2Fsome%2Fpage',
+            response.request.url)
 
-    def test_mantis_opener_can_handle_cookies(self):
-        # Ensure that the OpenerDirector of the Mantis bug tracker
+    @responses.activate
+    def test_mantis_makeRequest_can_handle_cookies(self):
+        # Ensure that the makeRequest method of the Mantis bug tracker
         # handles cookies.
+        responses.add(
+            'GET', 'http://mantis.example.com/', status=200,
+            headers={'Set-Cookie': 'foo=bar'})
         tracker = Mantis('http://mantis.example.com')
-        test_handler = UrlLib2TransportTestHandler()
-        opener = tracker.url_opener
-        opener.add_handler(test_handler)
-        opener.open('http://mantis.example.com', '')
-        cookies = list(tracker._cookie_handler.cookiejar)
-        self.assertEqual(1, len(cookies))
-        self.assertEqual('foo', cookies[0].name)
-        self.assertEqual('bar', cookies[0].value)
+        tracker.makeRequest('GET', 'http://mantis.example.com/')
+        self.assertThat(tracker._cookie_jar, MatchesListwise([
+            MatchesStructure.byEquality(name='foo', value='bar'),
+            ]))
 
+    @responses.activate
     def test_mantis_csv_file_http_500_error(self):
         # If a Mantis bug tracker returns a HTTP 500 error when the
         # URL for CSV data is accessed, we treat this as an
         # indication that we should screen scrape the bug data and
         # thus set csv_data to None.
+        responses.add(
+            'POST', 'http://mantis.example.com/view_all_set.php', status=200)
+        responses.add(
+            'GET', 'http://mantis.example.com/csv_export.php', status=500)
         tracker = Mantis('http://mantis.example.com')
-        test_handler = UrlLib2TransportTestHandler()
-        opener = tracker.url_opener
-        opener.add_handler(test_handler)
-        test_handler.setError(
-            HTTPError(
-                'http://mantis.example.com/csv_export.php', 500,
-                'Internal Error', {}, None),
-            'http://mantis.example.com/csv_export.php')
-        self.assertIs(None, tracker.csv_data)
+        self.assertIsNone(tracker.csv_data)
 
+    @responses.activate
     def test_mantis_csv_file_other_http_errors(self):
         # If the Mantis server returns other HTTP errors than 500,
         # they appear as BugTrackerConnectErrors.
+        responses.add(
+            'POST', 'http://mantis.example.com/view_all_set.php', status=200)
+        responses.add(
+            'GET', 'http://mantis.example.com/csv_export.php', status=503)
         tracker = Mantis('http://mantis.example.com')
-        test_handler = UrlLib2TransportTestHandler()
-        opener = tracker.url_opener
-        opener.add_handler(test_handler)
-        test_handler.setError(
-            HTTPError(
-                'http://mantis.example.com/csv_export.php', 503,
-                'Service Unavailable', {}, None),
-            'http://mantis.example.com/csv_export.php')
         self.assertRaises(BugTrackerConnectError, tracker._csv_data)
 
-        test_handler.setError(
-            HTTPError(
-                'http://mantis.example.com/csv_export.php', 404,
-                'Not Found', {}, None),
-            'http://mantis.example.com/csv_export.php')
+        responses.remove('GET', 'http://mantis.example.com/csv_export.php')
+        responses.add(
+            'GET', 'http://mantis.example.com/csv_export.php', status=404)
         self.assertRaises(BugTrackerConnectError, tracker._csv_data)
 
 


Follow ups