launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/explicit-proxy-sourceforge/+merge/348430
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/explicit-proxy-sourceforge into lp:launchpad.

=== modified file 'lib/lp/bugs/doc/externalbugtracker-sourceforge.txt'
--- lib/lp/bugs/doc/externalbugtracker-sourceforge.txt	2012-12-26 01:32:19 +0000
+++ lib/lp/bugs/doc/externalbugtracker-sourceforge.txt	2018-06-23 00:21:34 +0000
@@ -78,14 +78,14 @@
 it a set of remote bug IDs will fetch those bug IDs from the server and
 file them in a local variable for later use.
 
-We use a test-oriented implementation for the purposes of these tests,
-which overrides ExternalBugTracker.urlopen() so that we don't have to
-rely on a working network connection.
+We use a test-oriented implementation for the purposes of these tests, which
+avoids relying on a working network connection.
 
     >>> from lp.bugs.tests.externalbugtracker import (
     ...     TestSourceForge, print_bugwatches)
     >>> sourceforge = TestSourceForge('http://example.com/')
-    >>> sourceforge.initializeRemoteBugDB([1722250])
+    >>> with sourceforge.responses():
+    ...     sourceforge.initializeRemoteBugDB([1722250])
     >>> sorted(sourceforge.bugs.keys())
     [1722250]
 
@@ -93,7 +93,8 @@
 raised. We use a special sample bug, bug 0, which defines no status or
 resolution, to demonstrate this:
 
-    >>> sourceforge.initializeRemoteBugDB([0])
+    >>> with sourceforge.responses():
+    ...     sourceforge.initializeRemoteBugDB([0])
     Traceback (most recent call last):
       ...
     UnparsableBugData: Remote bug 0 does not define a status.
@@ -102,7 +103,8 @@
 status from them, we don't raise an error when trying to initialize the
 remote bug database. Sample bug 99 is private.
 
-    >>> sourceforge.initializeRemoteBugDB([99])
+    >>> with sourceforge.responses():
+    ...     sourceforge.initializeRemoteBugDB([99])
     >>> sorted(sourceforge.bugs.keys())
     [99]
 
@@ -168,8 +170,9 @@
     >>> bug_watch_updater = CheckwatchesMaster(
     ...     txn, logger=FakeLogger())
     >>> sourceforge = TestSourceForge(example_bug_tracker.baseurl)
-    >>> bug_watch_updater.updateBugWatches(
-    ...     sourceforge, example_bug_tracker.watches)
+    >>> with sourceforge.responses():
+    ...     bug_watch_updater.updateBugWatches(
+    ...         sourceforge, example_bug_tracker.watches)
     INFO Updating 1 watches for 1 bugs on http://bugs.some.where
     >>> print_bugwatches(example_bug_tracker.watches)
     Remote bug 1722250: Open:None
@@ -209,29 +212,30 @@
 
     >>> transaction.commit()
 
-    >>> sourceforge.trace_calls = True
-    >>> bug_watch_updater.updateBugWatches(
-    ...     sourceforge, sorted(example_bug_tracker.watches))
+    >>> with sourceforge.responses(trace_calls=True):
+    ...     bug_watch_updater.updateBugWatches(
+    ...         sourceforge, sorted(example_bug_tracker.watches))
     INFO Updating 1 watches for 1 bugs on http://bugs.some.where
-    CALLED _getPage(u'support/tracker.php?aid=1722251')
+    GET http://bugs.some.where/support/tracker.php?aid=1722251
 
 For the sake of this test we can set the bug tracker's batch_size to
 None so that it will process all the updates at once:
 
     >>> sourceforge.batch_size = None
-    >>> bug_watch_updater.updateBugWatches(
-    ...     sourceforge, example_bug_tracker.watches)
+    >>> with sourceforge.responses(trace_calls=True):
+    ...     bug_watch_updater.updateBugWatches(
+    ...         sourceforge, example_bug_tracker.watches)
     INFO Updating 10 watches for 10 bugs on http://bugs.some.where
-    CALLED _getPage(u'support/tracker.php?aid=1722250')
-    CALLED _getPage(u'support/tracker.php?aid=1722251')
-    CALLED _getPage(u'support/tracker.php?aid=1722252')
-    CALLED _getPage(u'support/tracker.php?aid=1722253')
-    CALLED _getPage(u'support/tracker.php?aid=1722254')
-    CALLED _getPage(u'support/tracker.php?aid=1722255')
-    CALLED _getPage(u'support/tracker.php?aid=1722256')
-    CALLED _getPage(u'support/tracker.php?aid=1722257')
-    CALLED _getPage(u'support/tracker.php?aid=1722258')
-    CALLED _getPage(u'support/tracker.php?aid=1722259')
+    GET http://bugs.some.where/support/tracker.php?aid=1722250
+    GET http://bugs.some.where/support/tracker.php?aid=1722251
+    GET http://bugs.some.where/support/tracker.php?aid=1722252
+    GET http://bugs.some.where/support/tracker.php?aid=1722253
+    GET http://bugs.some.where/support/tracker.php?aid=1722254
+    GET http://bugs.some.where/support/tracker.php?aid=1722255
+    GET http://bugs.some.where/support/tracker.php?aid=1722256
+    GET http://bugs.some.where/support/tracker.php?aid=1722257
+    GET http://bugs.some.where/support/tracker.php?aid=1722258
+    GET http://bugs.some.where/support/tracker.php?aid=1722259
 
 The bug statuses have now been imported from the Example.com bug
 tracker, so the bug watches should now have valid Launchpad bug
@@ -276,7 +280,9 @@
 If SourceForge can't find the group_id and atid for the bug (for example
 if the bug is private), getRemoteProduct() will return None.
 
-    >>> sourceforge.trace_calls = False
-    >>> sourceforge.initializeRemoteBugDB([99])
+    >>> transaction.commit()
+
+    >>> with sourceforge.responses():
+    ...     sourceforge.initializeRemoteBugDB([99])
     >>> print sourceforge.getRemoteProduct(99)
     None

=== modified file 'lib/lp/bugs/externalbugtracker/sourceforge.py'
--- lib/lp/bugs/externalbugtracker/sourceforge.py	2017-10-21 18:14:14 +0000
+++ lib/lp/bugs/externalbugtracker/sourceforge.py	2018-06-23 00:21:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 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).
 
 """Sourceforge ExternalBugTracker utility."""
@@ -11,7 +11,7 @@
 
 from lp.bugs.externalbugtracker import (
     BugNotFound,
-    ExternalBugTracker,
+    ExternalBugTrackerRequests,
     InvalidBugId,
     LookupTree,
     PrivateRemoteBug,
@@ -27,7 +27,7 @@
 from lp.services.webapp import urlsplit
 
 
-class SourceForge(ExternalBugTracker):
+class SourceForge(ExternalBugTrackerRequests):
     """An ExternalBugTracker for SourceForge bugs."""
 
     # We only allow ourselves to update one SourceForge bug at a time to
@@ -48,7 +48,7 @@
 
         for bug_id in bug_ids:
             query_url = self.export_url % bug_id
-            page_data = self._getPage(query_url)
+            page_data = self._getPage(query_url).content
 
             soup = BeautifulSoup(page_data)
             status_tag = soup.find(text=re.compile('Status:'))

=== modified file 'lib/lp/bugs/tests/externalbugtracker.py'
--- lib/lp/bugs/tests/externalbugtracker.py	2018-06-23 00:21:33 +0000
+++ lib/lp/bugs/tests/externalbugtracker.py	2018-06-23 00:21:34 +0000
@@ -26,6 +26,7 @@
 import responses
 from six.moves.urllib_parse import (
     parse_qs,
+    urljoin,
     urlsplit,
     )
 from zope.component import getUtility
@@ -1608,26 +1609,23 @@
                 'GET', re.compile(re.escape(self.baseurl)), self._getCallback)
 
 
-class TestSourceForge(SourceForge):
-    """Test-oriented SourceForge ExternalBugTracker.
-
-    Overrides _getPage() so that access to SourceForge itself is not
-    required.
-    """
-
-    trace_calls = False
-
-    def _getPage(self, page):
-        if self.trace_calls:
-            print "CALLED _getPage(%r)" % (page)
-
-        page_re = re.compile('support/tracker.php\?aid=([0-9]+)')
-        bug_id = page_re.match(page).groups()[0]
-
-        file_path = os.path.join(
-            os.path.dirname(__file__), 'testfiles',
-            'sourceforge-sample-bug-%s.html' % bug_id)
-        return open(file_path, 'r').read()
+class TestSourceForge(BugTrackerResponsesMixin, SourceForge):
+    """Test-oriented SourceForge ExternalBugTracker."""
+
+    @staticmethod
+    def _getCallback(request):
+        url = urlsplit(request.url)
+        bug_id = parse_qs(url.query)['aid'][0]
+        body = read_test_file('sourceforge-sample-bug-%s.html' % bug_id)
+        return 200, {'Content-Type': 'text/html'}, body
+
+    def addResponses(self, requests_mock):
+        """Add test responses."""
+        requests_mock.add_callback(
+            'GET',
+            re.compile(
+                re.escape(urljoin(self.baseurl, 'support/tracker.php'))),
+            self._getCallback)
 
 
 class TestDebianBug(debbugs.Bug):