launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22656
[Merge] lp:~cjwatson/launchpad/explicit-proxy-bugzilla into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/explicit-proxy-bugzilla into lp:launchpad with lp:~cjwatson/launchpad/explicit-proxy-mantis as a prerequisite.
Commit message:
Convert the Bugzilla external bug tracker to urlfetch.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/explicit-proxy-bugzilla/+merge/348432
I had to write a new XML-RPC transport that uses requests. It's a relatively small variation on the urllib2-based one, and I'll delete that once I've converted its other user.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/explicit-proxy-bugzilla into lp:launchpad.
=== modified file 'lib/lp/bugs/doc/externalbugtracker-bugzilla-api.txt'
--- lib/lp/bugs/doc/externalbugtracker-bugzilla-api.txt 2018-06-05 12:18:58 +0000
+++ lib/lp/bugs/doc/externalbugtracker-bugzilla-api.txt 2018-06-23 00:58:53 +0000
@@ -51,9 +51,9 @@
The authorisation cookie will be stored in the auth_cookie property of
the XML-RPC transport.
- >>> test_transport.cookie_processor.cookiejar
- <...CookieJar[Cookie(version=0, name='Bugzilla_login'...),
- Cookie(version=0, name='Bugzilla_logincookie'...)]>
+ >>> test_transport.cookie_jar
+ <RequestsCookieJar[Cookie(version=0, name='Bugzilla_login'...),
+ Cookie(version=0, name='Bugzilla_logincookie'...)]>
Trying to log in to a Bugzilla instance for which we have no credentials
will raise an error:
=== modified file 'lib/lp/bugs/doc/externalbugtracker-bugzilla-lp-plugin.txt'
--- lib/lp/bugs/doc/externalbugtracker-bugzilla-lp-plugin.txt 2018-06-05 12:18:58 +0000
+++ lib/lp/bugs/doc/externalbugtracker-bugzilla-lp-plugin.txt 2018-06-23 00:58:53 +0000
@@ -78,9 +78,9 @@
The authorisation cookie will be stored in the auth_cookie property of
the XML-RPC transport.
- >>> test_transport.cookie_processor.cookiejar
- <...CookieJar[Cookie(version=0, name='Bugzilla_login'...),
- Cookie(version=0, name='Bugzilla_logincookie'...)]>
+ >>> test_transport.cookie_jar
+ <RequestsCookieJar[Cookie(version=0, name='Bugzilla_login'...),
+ Cookie(version=0, name='Bugzilla_logincookie'...)]>
The externalbugtracker.bugzilla module contains a decorator,
needs_authentication, which can be used to ensure that a
=== modified file 'lib/lp/bugs/doc/externalbugtracker-bugzilla-oddities.txt'
--- lib/lp/bugs/doc/externalbugtracker-bugzilla-oddities.txt 2012-04-12 11:38:44 +0000
+++ lib/lp/bugs/doc/externalbugtracker-bugzilla-oddities.txt 2018-06-23 00:58:53 +0000
@@ -21,7 +21,9 @@
>>> txn = LaunchpadZopelessLayer.txn
>>> mozilla_bugzilla = getUtility(IBugTrackerSet).getByName('mozilla.org')
>>> issuezilla = TestIssuezilla(mozilla_bugzilla.baseurl)
- >>> issuezilla._probe_version()
+ >>> transaction.commit()
+ >>> with issuezilla.responses(post=False):
+ ... issuezilla._probe_version()
(2, 11)
>>> for bug_watch in mozilla_bugzilla.watches:
... print "%s: %s %s" % (bug_watch.remotebug,
@@ -32,8 +34,9 @@
42: FUBAR BAZBAZ
>>> transaction.commit()
>>> bug_watch_updater = CheckwatchesMaster(txn, logger=FakeLogger())
- >>> bug_watch_updater.updateBugWatches(
- ... issuezilla, mozilla_bugzilla.watches)
+ >>> with issuezilla.responses():
+ ... bug_watch_updater.updateBugWatches(
+ ... issuezilla, mozilla_bugzilla.watches)
INFO Updating 4 watches for 3 bugs on https://bugzilla.mozilla.org
INFO Didn't find bug u'42' on https://bugzilla.mozilla.org
(local bugs: 1, 2).
@@ -58,7 +61,9 @@
a) The version is way old:
- >>> old_bugzilla._probe_version()
+ >>> transaction.commit()
+ >>> with old_bugzilla.responses(post=False):
+ ... old_bugzilla._probe_version()
(2, 10)
b) The tags are not prefixed with the bz: namespace:
@@ -72,7 +77,8 @@
We support them just fine:
>>> remote_bugs = ['42', '123543']
- >>> old_bugzilla.initializeRemoteBugDB(remote_bugs)
+ >>> with old_bugzilla.responses():
+ ... old_bugzilla.initializeRemoteBugDB(remote_bugs)
>>> for remote_bug in remote_bugs:
... print "%s: %s %s" % (
... remote_bug,
@@ -90,7 +96,9 @@
>>> from lp.bugs.tests.externalbugtracker import (
... TestWeirdBugzilla)
>>> weird_bugzilla = TestWeirdBugzilla(mozilla_bugzilla.baseurl)
- >>> weird_bugzilla._probe_version()
+ >>> transaction.commit()
+ >>> with weird_bugzilla.responses(post=False):
+ ... weird_bugzilla._probe_version()
(2, 20)
a) The bug status tag is <bz:status> and not <bz:bug_status>
@@ -109,7 +117,8 @@
Yet everything still works as expected:
>>> remote_bugs = ['2000', '123543']
- >>> weird_bugzilla.initializeRemoteBugDB(remote_bugs)
+ >>> with weird_bugzilla.responses():
+ ... weird_bugzilla.initializeRemoteBugDB(remote_bugs)
>>> for remote_bug in remote_bugs:
... print "%s: %s %s" % (
... remote_bug,
@@ -128,13 +137,16 @@
>>> from lp.bugs.tests.externalbugtracker import (
... TestBrokenBugzilla)
>>> broken_bugzilla = TestBrokenBugzilla(mozilla_bugzilla.baseurl)
- >>> broken_bugzilla._probe_version()
+ >>> transaction.commit()
+ >>> with broken_bugzilla.responses(post=False):
+ ... broken_bugzilla._probe_version()
(2, 20)
>>> "</foobar>" in broken_bugzilla._readBugItemFile()
True
>>> remote_bugs = ['42', '2000']
- >>> broken_bugzilla.initializeRemoteBugDB(remote_bugs)
+ >>> with broken_bugzilla.responses():
+ ... broken_bugzilla.initializeRemoteBugDB(remote_bugs)
Traceback (most recent call last):
...
UnparsableBugData: Failed to parse XML description...
@@ -148,4 +160,6 @@
True
>>> remote_bugs = ['42', '2000']
- >>> broken_bugzilla.initializeRemoteBugDB(remote_bugs) # no exception
+ >>> transaction.commit()
+ >>> with broken_bugzilla.responses():
+ ... broken_bugzilla.initializeRemoteBugDB(remote_bugs) # no exception
=== modified file 'lib/lp/bugs/doc/externalbugtracker-bugzilla.txt'
--- lib/lp/bugs/doc/externalbugtracker-bugzilla.txt 2017-10-24 08:11:47 +0000
+++ lib/lp/bugs/doc/externalbugtracker-bugzilla.txt 2018-06-23 00:58:53 +0000
@@ -63,7 +63,9 @@
>>> gnome_bugzilla = (
... getUtility(IBugTrackerSet).getByName('gnome-bugzilla'))
>>> external_bugzilla = TestBugzilla(gnome_bugzilla.baseurl)
- >>> version = external_bugzilla._probe_version()
+ >>> transaction.commit()
+ >>> with external_bugzilla.responses(post=False):
+ ... version = external_bugzilla._probe_version()
>>> version
(2, 20)
@@ -459,8 +461,9 @@
... bug_watch.remote_importance)
304070: None None
3224: None
- >>> bug_watch_updater.updateBugWatches(
- ... external_bugzilla, gnome_bugzilla.watches)
+ >>> with external_bugzilla.responses():
+ ... bug_watch_updater.updateBugWatches(
+ ... external_bugzilla, gnome_bugzilla.watches)
INFO Updating 2 watches for 2 bugs on http://bugzilla.gnome.org/bugs
INFO Didn't find bug u'304070' on
http://bugzilla.gnome.org/bugs (local bugs: 15).
@@ -503,19 +506,19 @@
Then updateBugWatches() will make one request per bug watch:
- >>> external_bugzilla.trace_calls = True
- >>> bug_watch_updater.updateBugWatches(
- ... external_bugzilla, gnome_bugzilla.watches)
+ >>> with external_bugzilla.responses(trace_calls=True, get=False):
+ ... bug_watch_updater.updateBugWatches(
+ ... external_bugzilla, gnome_bugzilla.watches)
INFO Updating 7 watches for 7 bugs on http://bugzilla.gnome.org/bugs
- CALLED _postPage()
- CALLED _postPage()
- CALLED _postPage()
- CALLED _postPage()
- CALLED _postPage()
- CALLED _postPage()
- CALLED _postPage()
INFO Didn't find bug u'304070' on
http://bugzilla.gnome.org/bugs (local bugs: 15).
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
>>> remote_statuses = dict(
... [(int(bug_watch.remotebug), bug_watch.remotestatus)
@@ -529,8 +532,6 @@
>>> remote_importances == expected_remote_importances
True
- >>> external_bugzilla.trace_calls = False
-
Let's add a few more watches:
>>> expected_remote_statuses = dict(
@@ -557,13 +558,13 @@
Instead of issuing one request per bug watch, like was done before,
updateBugWatches() issues only one request to update all watches:
- >>> external_bugzilla.trace_calls = True
- >>> bug_watch_updater.updateBugWatches(
- ... external_bugzilla, gnome_bugzilla.watches)
+ >>> with external_bugzilla.responses(trace_calls=True, get=False):
+ ... bug_watch_updater.updateBugWatches(
+ ... external_bugzilla, gnome_bugzilla.watches)
INFO Updating 207 watches for 207 bugs...
- CALLED _postPage()
INFO Didn't find bug u'304070' on
http://bugzilla.gnome.org/bugs (local bugs: 15).
+ POST http://bugzilla.gnome.org/bugs/buglist.cgi
>>> remote_statuses = dict(
... [(int(bug_watch.remotebug), bug_watch.remotestatus)
@@ -577,8 +578,6 @@
>>> remote_importances == expected_remote_importances
True
- >>> external_bugzilla.trace_calls = False
-
updateBugWatches() updates the lastchecked attribute on the watches, so
now no bug watches are in need of updating:
@@ -596,7 +595,8 @@
>>> now = datetime.now(pytz.timezone('UTC'))
>>> bug_watch.lastchanged = now - timedelta(weeks=2)
>>> old_last_changed = bug_watch.lastchanged
- >>> bug_watch_updater.updateBugWatches(external_bugzilla, [bug_watch])
+ >>> with external_bugzilla.responses(get=False):
+ ... bug_watch_updater.updateBugWatches(external_bugzilla, [bug_watch])
INFO Updating 1 watches for 1 bugs on http://bugzilla.gnome.org/bugs
>>> bug_watch.lastchanged == old_last_changed
True
@@ -632,8 +632,9 @@
Let's update the bug watch, and see that the linked bug watch got
synced:
- >>> bug_watch_updater.updateBugWatches(
- ... external_bugzilla, [thunderbird_task.bugwatch])
+ >>> with external_bugzilla.responses(get=False):
+ ... bug_watch_updater.updateBugWatches(
+ ... external_bugzilla, [thunderbird_task.bugwatch])
INFO Updating 1 watches for 1 bugs on https://bugzilla.mozilla.org
>>> bug_nine = getUtility(IBugSet).get(9)
@@ -655,8 +656,9 @@
>>> thunderbird_task.transitionToStatus(
... BugTaskStatus.CONFIRMED,
... getUtility(IPersonSet).getByName('no-priv'))
- >>> bug_watch_updater.updateBugWatches(
- ... external_bugzilla, [thunderbird_task.bugwatch])
+ >>> with external_bugzilla.responses(get=False):
+ ... bug_watch_updater.updateBugWatches(
+ ... external_bugzilla, [thunderbird_task.bugwatch])
INFO Updating 1 watches for 1 bugs on https://bugzilla.mozilla.org
>>> bug_nine = getUtility(IBugSet).get(9)
@@ -684,8 +686,9 @@
... bug=bug_two, owner=sample_person, bugtracker=mozilla_bugzilla,
... remotebug='42')
>>> bug_watch2_id = bug_watch2.id
- >>> bug_watch_updater.updateBugWatches(
- ... external_bugzilla, [bug_watch1, bug_watch2])
+ >>> with external_bugzilla.responses(get=False):
+ ... bug_watch_updater.updateBugWatches(
+ ... external_bugzilla, [bug_watch1, bug_watch2])
INFO Updating 2 watches for 1 bugs on https://bugzilla.mozilla.org
>>> bug_watch1 = getUtility(IBugWatchSet).get(bug_watch1_id)
@@ -702,10 +705,12 @@
If updateBugWatches() can't parse the XML file returned from the remote
bug tracker, an error is logged.
- >>> external_bugzilla._postPage = (
- ... lambda self, data, repost_on_redirect: '<invalid xml>')
- >>> bug_watch_updater.updateBugWatches(
- ... external_bugzilla, [bug_watch1, bug_watch2])
+ >>> import re
+ >>> with external_bugzilla.responses() as requests_mock:
+ ... requests_mock.reset()
+ ... requests_mock.add('POST', re.compile(r'.*'), body='<invalid xml>')
+ ... bug_watch_updater.updateBugWatches(
+ ... external_bugzilla, [bug_watch1, bug_watch2])
Traceback (most recent call last):
...
UnparsableBugData:
@@ -730,10 +735,13 @@
initializeRemoteBugDB() has been called, in order for the bug
information to be fetched from the external Bugzilla instance.
+ >>> transaction.commit()
+
>>> external_bugzilla = TestBugzilla()
>>> external_bugzilla.bugzilla_bugs = {84: (
... 'RESOLVED', 'FIXED', 'MEDIUM', 'NORMAL')}
- >>> external_bugzilla.initializeRemoteBugDB(['84'])
+ >>> with external_bugzilla.responses():
+ ... external_bugzilla.initializeRemoteBugDB(['84'])
>>> external_bugzilla.remote_bug_product['84']
u'product-84'
>>> external_bugzilla.getRemoteProduct('84')
@@ -747,7 +755,8 @@
... 'RESOLVED', 'FIXED', 'MEDIUM', 'NORMAL')}
>>> # Make the buglist XML not include the product tag.
>>> external_bugzilla.bug_item_file = 'gnome_bug_li_item_noproduct.xml'
- >>> external_bugzilla.initializeRemoteBugDB(['84'])
+ >>> with external_bugzilla.responses():
+ ... external_bugzilla.initializeRemoteBugDB(['84'])
>>> print external_bugzilla.getRemoteProduct('84')
None
@@ -756,7 +765,8 @@
>>> external_bugzilla = TestBugzilla()
>>> external_bugzilla.bugzilla_bugs = {84: (
... 'RESOLVED', 'FIXED', 'MEDIUM', 'NORMAL')}
- >>> external_bugzilla.initializeRemoteBugDB(['84'])
+ >>> with external_bugzilla.responses():
+ ... external_bugzilla.initializeRemoteBugDB(['84'])
>>> external_bugzilla.getRemoteProduct('42')
Traceback (most recent call last):
...
=== modified file 'lib/lp/bugs/externalbugtracker/bugzilla.py'
--- lib/lp/bugs/externalbugtracker/bugzilla.py 2018-06-05 12:18:58 +0000
+++ lib/lp/bugs/externalbugtracker/bugzilla.py 2018-06-23 00:58:53 +0000
@@ -15,12 +15,12 @@
from httplib import BadStatusLine
import re
import string
-from urllib2 import URLError
from xml.dom import minidom
import xml.parsers.expat
import xmlrpclib
import pytz
+import requests
import six
from zope.component import getUtility
from zope.interface import (
@@ -32,7 +32,7 @@
BugNotFound,
BugTrackerAuthenticationError,
BugTrackerConnectError,
- ExternalBugTracker,
+ ExternalBugTrackerRequests,
InvalidBugId,
LookupTree,
UnknownRemoteImportanceError,
@@ -40,7 +40,7 @@
UnparsableBugData,
UnparsableBugTrackerVersion,
)
-from lp.bugs.externalbugtracker.xmlrpc import UrlLib2Transport
+from lp.bugs.externalbugtracker.xmlrpc import RequestsTransport
from lp.bugs.interfaces.bugtask import (
BugTaskImportance,
BugTaskStatus,
@@ -61,8 +61,8 @@
)
-class Bugzilla(ExternalBugTracker):
- """An ExternalBugTrack for dealing with remote Bugzilla systems."""
+class Bugzilla(ExternalBugTrackerRequests):
+ """An ExternalBugTracker for dealing with remote Bugzilla systems."""
batch_query_threshold = 0 # Always use the batch method.
_test_xmlrpc_proxy = None
@@ -171,7 +171,8 @@
return BugzillaLPPlugin(self.baseurl)
elif self._remoteSystemHasBugzillaAPI():
return BugzillaAPI(self.baseurl)
- except (xmlrpclib.ProtocolError, URLError, BadStatusLine):
+ except (xmlrpclib.ProtocolError, requests.RequestException,
+ BadStatusLine):
pass
return self
@@ -201,7 +202,7 @@
server cannot be reached `BugTrackerConnectError` will be
raised.
"""
- version_xml = self._getPage('xml.cgi?id=1')
+ version_xml = self._getPage('xml.cgi?id=1').content
try:
document = self._parseDOMString(version_xml)
except xml.parsers.expat.ExpatError as e:
@@ -410,7 +411,7 @@
severity_tag = 'bz:bug_severity'
buglist_xml = self._postPage(
- buglist_page, data, repost_on_redirect=True)
+ buglist_page, data, repost_on_redirect=True).content
try:
document = self._parseDOMString(buglist_xml)
@@ -568,7 +569,7 @@
self.internal_xmlrpc_transport = internal_xmlrpc_transport
if xmlrpc_transport is None:
- self.xmlrpc_transport = UrlLib2Transport(self.xmlrpc_endpoint)
+ self.xmlrpc_transport = RequestsTransport(self.xmlrpc_endpoint)
else:
self.xmlrpc_transport = xmlrpc_transport
=== modified file 'lib/lp/bugs/externalbugtracker/tests/test_bugzilla.py'
--- lib/lp/bugs/externalbugtracker/tests/test_bugzilla.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/externalbugtracker/tests/test_bugzilla.py 2018-06-23 00:58:53 +0000
@@ -1,14 +1,14 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the Bugzilla BugTracker."""
__metaclass__ = type
-from StringIO import StringIO
from xml.parsers.expat import ExpatError
import xmlrpclib
+import responses
import transaction
from lp.bugs.externalbugtracker.base import UnparsableBugData
@@ -17,7 +17,6 @@
TestCase,
TestCaseWithFactory,
)
-from lp.testing.fakemethod import FakeMethod
from lp.testing.layers import ZopelessLayer
@@ -28,28 +27,26 @@
base_url = "http://example.com/";
- def _makeInstrumentedBugzilla(self, page=None, content=None):
- """Create a `Bugzilla` with a fake urlopen."""
- if page is None:
- page = self.factory.getUniqueString()
- bugzilla = Bugzilla(self.base_url)
- if content is None:
- content = "<bugzilla>%s</bugzilla>" % (
- self.factory.getUniqueString())
- fake_page = StringIO(content)
- fake_page.url = self.base_url + page
- bugzilla.urlopen = FakeMethod(result=fake_page)
- return bugzilla
-
+ @responses.activate
def test_post_to_search_form_does_not_crash(self):
- page = self.factory.getUniqueString()
- bugzilla = self._makeInstrumentedBugzilla(page)
+ responses.add(
+ "POST", self.base_url + "xml.cgi",
+ body="<bugzilla>%s</bugzilla>" % self.factory.getUniqueString())
+ bugzilla = Bugzilla(self.base_url)
bugzilla.getRemoteBugBatch([])
+ @responses.activate
def test_repost_on_redirect_does_not_crash(self):
- bugzilla = self._makeInstrumentedBugzilla()
+ responses.add(
+ "POST", self.base_url + "xml.cgi", status=302,
+ headers={"Location": self.base_url + "buglist.cgi"})
+ responses.add(
+ "POST", self.base_url + "buglist.cgi",
+ body="<bugzilla>%s</bugzilla>" % self.factory.getUniqueString())
+ bugzilla = Bugzilla(self.base_url)
bugzilla.getRemoteBugBatch([])
+ @responses.activate
def test_reports_invalid_search_result(self):
# Sometimes bug searches may go wrong, yielding an HTML page
# instead. getRemoteBugBatch rejects and reports search results
@@ -61,7 +58,8 @@
</body>
</html>
"""
- bugzilla = self._makeInstrumentedBugzilla(content=result_text)
+ responses.add("POST", self.base_url + "xml.cgi", body=result_text)
+ bugzilla = Bugzilla(self.base_url)
self.assertRaises(UnparsableBugData, bugzilla.getRemoteBugBatch, [])
=== modified file 'lib/lp/bugs/externalbugtracker/tests/test_xmlrpc.py'
--- lib/lp/bugs/externalbugtracker/tests/test_xmlrpc.py 2018-04-05 16:18:34 +0000
+++ lib/lp/bugs/externalbugtracker/tests/test_xmlrpc.py 2018-06-23 00:58:53 +0000
@@ -1,4 +1,4 @@
-# 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 `lp.bugs.externalbugtracker.xmlrpc`."""
@@ -10,8 +10,13 @@
from xml.parsers.expat import ExpatError
from fixtures import MockPatch
+import requests
+import responses
-from lp.bugs.externalbugtracker.xmlrpc import UrlLib2Transport
+from lp.bugs.externalbugtracker.xmlrpc import (
+ RequestsTransport,
+ UrlLib2Transport,
+ )
from lp.bugs.tests.externalbugtracker import (
ensure_response_parser_is_expat,
UrlLib2TransportTestHandler,
@@ -51,3 +56,41 @@
URLError, '<urlopen error [Errno -2] Name or service not known>',
transport.request, u"test.invalid", u"xmlrpc",
u"\N{SNOWMAN}".encode('utf-8'))
+
+
+class TestRequestsTransport(TestCase):
+ """Tests for `RequestsTransport`."""
+
+ @responses.activate
+ def test_expat_error(self):
+ # Malformed XML-RPC responses cause xmlrpclib to raise an ExpatError.
+ responses.add(
+ "POST", "http://www.example.com/xmlrpc";,
+ body="<params><mis></match></params>")
+ transport = RequestsTransport("http://not.real/";)
+
+ # The Launchpad production environment selects Expat at present. This
+ # is quite strict compared to the other parsers that xmlrpclib can
+ # possibly select.
+ ensure_response_parser_is_expat(transport)
+
+ self.assertRaises(
+ ExpatError, transport.request,
+ 'www.example.com', 'xmlrpc', "<methodCall />")
+
+ def test_unicode_url(self):
+ # Python's httplib doesn't like Unicode URLs much. Ensure that
+ # they don't cause it to crash, and we get a post-serialisation
+ # connection error instead.
+ self.useFixture(MockPatch(
+ "socket.getaddrinfo",
+ side_effect=socket.gaierror(
+ socket.EAI_NONAME, "Name or service not known")))
+ transport = RequestsTransport(u"http://test.invalid/";)
+ for proxy in (None, "http://squid.internal:3128/";):
+ self.pushConfig("launchpad", http_proxy=proxy)
+ e = self.assertRaises(
+ requests.ConnectionError,
+ transport.request, u"test.invalid", u"xmlrpc",
+ u"\N{SNOWMAN}".encode('utf-8'))
+ self.assertIn("Name or service not known", str(e))
=== modified file 'lib/lp/bugs/externalbugtracker/xmlrpc.py'
--- lib/lp/bugs/externalbugtracker/xmlrpc.py 2015-07-10 07:48:06 +0000
+++ lib/lp/bugs/externalbugtracker/xmlrpc.py 2018-06-23 00:58:53 +0000
@@ -1,10 +1,11 @@
-# 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).
-"""An XMLRPC transport which uses urllib2."""
+"""XMLRPC transports which use urllib2 or requests."""
__metaclass__ = type
__all__ = [
+ 'RequestsTransport',
'UrlLib2Transport',
'XMLRPCRedirectHandler',
]
@@ -12,6 +13,7 @@
from cookielib import Cookie
from cStringIO import StringIO
+from io import BytesIO
from urllib2 import (
build_opener,
HTTPCookieProcessor,
@@ -28,7 +30,15 @@
Transport,
)
+import requests
+from requests.cookies import RequestsCookieJar
+
+from lp.bugs.externalbugtracker.base import repost_on_redirect_hook
from lp.services.config import config
+from lp.services.timeout import (
+ override_timeout,
+ urlfetch,
+ )
from lp.services.utils import traceback_info
@@ -122,3 +132,61 @@
else:
traceback_info(response)
return self.parse_response(StringIO(response))
+
+
+class RequestsTransport(Transport):
+ """An XML-RPC transport which uses requests.
+
+ This XML-RPC transport uses the Python requests module to make the
+ request. (In fact, it uses lp.services.timeout.urlfetch, which wraps
+ requests and deals with timeout handling.)
+
+ Note: this transport isn't fit for general XML-RPC use. It is just good
+ enough for some of our external bug tracker implementations.
+
+ :param endpoint: The URL of the XML-RPC server.
+ """
+
+ verbose = False
+
+ def __init__(self, endpoint, cookie_jar=None):
+ Transport.__init__(self, use_datetime=True)
+ self.scheme, self.host = urlparse(endpoint)[:2]
+ assert self.scheme in ('http', 'https'), (
+ "Unsupported URL scheme: %s" % self.scheme)
+ if cookie_jar is None:
+ cookie_jar = RequestsCookieJar()
+ self.cookie_jar = cookie_jar
+ self.timeout = config.checkwatches.default_socket_timeout
+
+ def setCookie(self, cookie_str):
+ """Set a cookie for the transport to use in future connections."""
+ name, value = cookie_str.split('=')
+ self.cookie_jar.set(
+ name, value, domain=self.host, path='', expires=False,
+ discard=None, rest=None)
+
+ def request(self, host, handler, request_body, verbose=0):
+ """Make an XMLRPC request.
+
+ Uses the configured proxy server to make the connection.
+ """
+ url = urlunparse((self.scheme, host, handler, '', '', ''))
+ # httplib can raise a UnicodeDecodeError when using a Unicode
+ # URL, a non-ASCII body and a proxy. http://bugs.python.org/issue12398
+ if not isinstance(url, bytes):
+ url = url.encode('utf-8')
+ try:
+ with override_timeout(self.timeout):
+ response = urlfetch(
+ url, method='POST', headers={'Content-Type': 'text/xml'},
+ data=request_body, cookies=self.cookie_jar,
+ hooks={'response': repost_on_redirect_hook},
+ trust_env=False, use_proxy=True)
+ except requests.HTTPError as e:
+ raise ProtocolError(
+ url.decode('utf-8'), e.response.status_code, e.response.reason,
+ e.response.headers)
+ else:
+ traceback_info(response.text)
+ return self.parse_response(BytesIO(response.content))
=== modified file 'lib/lp/bugs/tests/bugzilla-api-xmlrpc-transport.txt'
--- lib/lp/bugs/tests/bugzilla-api-xmlrpc-transport.txt 2018-06-05 12:18:58 +0000
+++ lib/lp/bugs/tests/bugzilla-api-xmlrpc-transport.txt 2018-06-23 00:58:53 +0000
@@ -43,9 +43,9 @@
The Bugzilla_logincookie will now have been set for the transport, too.
- >>> print bugzilla_transport.cookie_processor.cookiejar
- <...CookieJar[<Cookie Bugzilla_login=...>,
- <Cookie Bugzilla_logincookie=...>]>
+ >>> print bugzilla_transport.cookie_jar
+ <RequestsCookieJar[<Cookie Bugzilla_login=...>,
+ <Cookie Bugzilla_logincookie=...>]>
Trying to log in with an incorrect username or password will result in
an error being raised.
=== modified file 'lib/lp/bugs/tests/bugzilla-xmlrpc-transport.txt'
--- lib/lp/bugs/tests/bugzilla-xmlrpc-transport.txt 2018-06-05 12:18:58 +0000
+++ lib/lp/bugs/tests/bugzilla-xmlrpc-transport.txt 2018-06-23 00:58:53 +0000
@@ -90,9 +90,9 @@
The login cookies are in the transport's cookie jar.
- >>> print bugzilla_transport.cookie_processor.cookiejar
- <...CookieJar[<Cookie Bugzilla_login=...>,
- <Cookie Bugzilla_logincookie=...>]>
+ >>> print bugzilla_transport.cookie_jar
+ <RequestsCookieJar[<Cookie Bugzilla_login=...>,
+ <Cookie Bugzilla_logincookie=...>]>
Launchpad.time()
=== modified file 'lib/lp/bugs/tests/externalbugtracker-xmlrpc-transport.txt'
--- lib/lp/bugs/tests/externalbugtracker-xmlrpc-transport.txt 2016-09-21 02:49:42 +0000
+++ lib/lp/bugs/tests/externalbugtracker-xmlrpc-transport.txt 2018-06-23 00:58:53 +0000
@@ -193,3 +193,125 @@
>>> print redirected_request.data
None
+
+
+XMLRPC requests transport
+-------------------------
+
+When using XMLRPC for connecting to external bug trackers, we need to
+use a special transport, which processes http cookies correctly, and
+which can connect through an http proxy server.
+
+ >>> from lp.bugs.externalbugtracker.xmlrpc import RequestsTransport
+
+RequestsTransport accepts a CookieJar as an optional parameter upon creation.
+This allows us to share a CookieJar - and therefore the cookie it contains -
+between different transports or URL openers.
+
+ >>> from requests.cookies import RequestsCookieJar
+ >>> jar = RequestsCookieJar()
+ >>> transport = RequestsTransport('http://example.com', jar)
+ >>> transport.cookie_jar == jar
+ True
+
+We define a test response callback that returns the request-url and any
+request parameters as an XMLRPC parameter, and sets a cookie from the
+server, 'foo=bar'.
+
+ >>> import responses
+ >>> from six.moves import xmlrpc_client
+
+ >>> def test_callback(request):
+ ... params = xmlrpc_client.loads(request.body)[0]
+ ... return (
+ ... 200, {'Set-Cookie': 'foo=bar'},
+ ... xmlrpc_client.dumps(
+ ... ([request.url] + list(params),), methodresponse=True))
+
+Before sending the request, the transport's cookie jar is empty.
+
+ >>> def print_cookie_jar(jar):
+ ... for name, value in sorted(jar.items()):
+ ... print('%s=%s' % (name, value))
+
+ >>> print_cookie_jar(transport.cookie_jar)
+
+ >>> request_body = """<?xml version="1.0"?>
+ ... <methodCall>
+ ... <methodName>examples.testMethod</methodName>
+ ... <params>
+ ... <param>
+ ... <value>
+ ... <int>42</int>
+ ... </value>
+ ... </param>
+ ... </params>
+ ... </methodCall>
+ ... """
+ >>> with responses.RequestsMock() as requests_mock:
+ ... requests_mock.add_callback(
+ ... 'POST', 'http://www.example.com/xmlrpc', test_callback)
+ ... transport.request('www.example.com', 'xmlrpc', request_body)
+ (['http://www.example.com/xmlrpc', 42],)
+
+We received the url as the single XMLRPC result, and the cookie jar now
+contains the 'foo=bar' cookie sent by the server.
+
+ >>> print_cookie_jar(transport.cookie_jar)
+ foo=bar
+
+In addition to cookies sent by the server, we can set cookies locally.
+
+ >>> transport.setCookie('ding=dong')
+ >>> print_cookie_jar(transport.cookie_jar)
+ ding=dong
+ foo=bar
+
+If an error occurs trying to make the request, an
+``xmlrpclib.ProtocolError`` is raised.
+
+ >>> request_body = """<?xml version="1.0"?>
+ ... <methodCall>
+ ... <methodName>examples.testError</methodName>
+ ... <params>
+ ... <param>
+ ... <value>
+ ... <int>42</int>
+ ... </value>
+ ... </param>
+ ... </params>
+ ... </methodCall>
+ ... """
+ >>> with responses.RequestsMock() as requests_mock:
+ ... requests_mock.add(
+ ... 'POST', 'http://www.example.com/xmlrpc', status=500)
+ ... transport.request('www.example.com', 'xmlrpc', request_body)
+ Traceback (most recent call last):
+ ...
+ ProtocolError: <ProtocolError for http://www.example.com/xmlrpc: 500
+ Internal Server Error>
+
+If the transport encounters a redirect response it will make its request
+to the location indicated in that response rather than the original
+location.
+
+ >>> request_body = """<?xml version="1.0"?>
+ ... <methodCall>
+ ... <methodName>examples.whatever</methodName>
+ ... <params>
+ ... <param>
+ ... <value>
+ ... <int>42</int>
+ ... </value>
+ ... </param>
+ ... </params>
+ ... </methodCall>
+ ... """
+ >>> with responses.RequestsMock() as requests_mock:
+ ... target_url = 'http://www.example.com/xmlrpc/redirected'
+ ... requests_mock.add(
+ ... 'POST', 'http://www.example.com/xmlrpc', status=302,
+ ... headers={'Location': target_url})
+ ... requests_mock.add_callback('POST', target_url, test_callback)
+ ... transport.request('www.example.com', 'xmlrpc', request_body)
+ (['http://www.example.com/xmlrpc/redirected', 42],)
=== modified file 'lib/lp/bugs/tests/externalbugtracker.py'
--- lib/lp/bugs/tests/externalbugtracker.py 2018-06-23 00:58:53 +0000
+++ lib/lp/bugs/tests/externalbugtracker.py 2018-06-23 00:58:53 +0000
@@ -52,7 +52,10 @@
LP_PLUGIN_METADATA_AND_COMMENTS,
LP_PLUGIN_METADATA_ONLY,
)
-from lp.bugs.externalbugtracker.xmlrpc import UrlLib2Transport
+from lp.bugs.externalbugtracker.xmlrpc import (
+ RequestsTransport,
+ UrlLib2Transport,
+ )
from lp.bugs.interfaces.bugtask import (
BugTaskImportance,
BugTaskStatus,
@@ -260,12 +263,8 @@
raise self.get_remote_status_error("Testing")
-class TestBugzilla(Bugzilla):
- """Bugzilla ExternalSystem for use in tests.
-
- It overrides _getPage and _postPage, so that access to a real Bugzilla
- instance isn't needed.
- """
+class TestBugzilla(BugTrackerResponsesMixin, Bugzilla):
+ """Bugzilla ExternalSystem for use in tests."""
# We set the batch_query_threshold to zero so that only
# getRemoteBugBatch() is used to retrieve bugs, since getRemoteBug()
# calls getRemoteBugBatch() anyway.
@@ -301,40 +300,38 @@
"""
return read_test_file(self.bug_item_file)
- def _getPage(self, page):
- """GET a page.
+ def _getCallback(self, request):
+ """Handle a test GET request.
Only handles xml.cgi?id=1 so far.
"""
- if self.trace_calls:
- print "CALLED _getPage()"
- if page == 'xml.cgi?id=1':
- data = read_test_file(self.version_file)
+ url = urlsplit(request.url)
+ if (url.path == urlsplit(self.baseurl).path + '/xml.cgi' and
+ parse_qs(url.query).get('id') == ['1']):
+ body = read_test_file(self.version_file)
# Add some latin1 to test bug 61129
- return data % dict(non_ascii_latin1="\xe9")
+ return 200, {}, body % {'non_ascii_latin1': b'\xe9'}
else:
- raise AssertionError('Unknown page: %s' % page)
-
- def _postPage(self, page, form, repost_on_redirect=False):
- """POST to the specified page.
-
- :form: is a dict of form variables being POSTed.
+ raise AssertionError('Unknown URL: %s' % request.url)
+
+ def _postCallback(self, request):
+ """Handle a test POST request.
Only handles buglist.cgi so far.
"""
- if self.trace_calls:
- print "CALLED _postPage()"
- if page == self.buglist_page:
+ url = urlsplit(request.url)
+ if url.path == urlsplit(self.baseurl).path + '/' + self.buglist_page:
buglist_xml = read_test_file(self.buglist_file)
- bug_ids = str(form[self.bug_id_form_element]).split(',')
+ form = parse_qs(request.body)
+ bug_ids = str(form[self.bug_id_form_element][0]).split(',')
bug_li_items = []
for bug_id in bug_ids:
bug_id = int(bug_id)
if bug_id not in self.bugzilla_bugs:
- #Unknown bugs aren't included in the resulting xml.
+ # Unknown bugs aren't included in the resulting xml.
continue
- bug_status, bug_resolution, bug_priority, bug_severity = \
- self.bugzilla_bugs[int(bug_id)]
+ bug_status, bug_resolution, bug_priority, bug_severity = (
+ self.bugzilla_bugs[int(bug_id)])
bug_item = self._readBugItemFile() % {
'bug_id': bug_id,
'status': bug_status,
@@ -343,12 +340,22 @@
'severity': bug_severity,
}
bug_li_items.append(bug_item)
- return buglist_xml % {
+ body = buglist_xml % {
'bug_li_items': '\n'.join(bug_li_items),
- 'page': page,
+ 'page': url.path.lstrip('/'),
}
+ return 200, {}, body
else:
- raise AssertionError('Unknown page: %s' % page)
+ raise AssertionError('Unknown URL: %s' % request.url)
+
+ def addResponses(self, requests_mock, get=True, post=True):
+ """Add test responses."""
+ if get:
+ requests_mock.add_callback(
+ 'GET', re.compile(r'.*'), self._getCallback)
+ if post:
+ requests_mock.add_callback(
+ 'POST', re.compile(r'.*'), self._postCallback)
class TestWeirdBugzilla(TestBugzilla):
@@ -406,14 +413,7 @@
123543: ('ASSIGNED', '', 'HIGH', 'BLOCKER')}
-class FakeHTTPConnection:
- """A fake HTTP connection."""
-
- def putheader(self, header, value):
- print "%s: %s" % (header, value)
-
-
-class TestBugzillaXMLRPCTransport(UrlLib2Transport):
+class TestBugzillaXMLRPCTransport(RequestsTransport):
"""A test implementation of the Bugzilla XML-RPC interface."""
local_datetime = None
@@ -538,9 +538,9 @@
def __init__(self, *args, **kwargs):
"""Ensure mutable class data is copied to the instance."""
- # UrlLib2Transport is not a new style class so 'super' cannot be
+ # RequestsTransport is not a new-style class so 'super' cannot be
# used.
- UrlLib2Transport.__init__(self, *args, **kwargs)
+ RequestsTransport.__init__(self, *args, **kwargs)
self.bugs = deepcopy(TestBugzillaXMLRPCTransport._bugs)
self.bug_aliases = deepcopy(self._bug_aliases)
self.bug_comments = deepcopy(self._bug_comments)
@@ -551,14 +551,13 @@
@property
def auth_cookie(self):
- cookies = self.cookie_processor.cookiejar._cookies
-
- assert len(cookies) < 2, (
- "There should only be cookies for one domain.")
-
- if len(cookies) == 1:
- [(domain, domain_cookies)] = cookies.items()
- return domain_cookies.get('', {}).get('Bugzilla_logincookie')
+ if len(set(cookie.domain for cookie in self.cookie_jar)) > 1:
+ raise AssertionError(
+ "There should only be cookies for one domain.")
+
+ for cookie in self.cookie_jar:
+ if cookie.name == 'Bugzilla_logincookie':
+ return cookie
else:
return None
Follow ups
