Merge lp:~allenap/launchpad/kde-bugzilla-being-odd into lp:launchpad

[Gavin Panella]

Don't break when an XML-RPC reply from a remote Bugzilla instance is not a mapping. Older Bugzilla instances return tuples. Also, simplify Bugzilla version parsing.

[Gavin Panella (allenap)]

KDE bugs returns tuples instead of mappings to XML-RPC calls. This causes the Bugzilla sniffer to fall over. I've changed it to consider anything other than a dict as unsupported (for API consideration). This means Launchpad will fall-back to the standard boring stupid Bugzilla support, which should work at least. This addresses bug 505958. I discussed this with Graham Binns briefly before implementation.

I also noticed that I could have a go at fixing Bugzilla._parseVersion() to simply extract the numeric parts of the version. This addresses bug 334980.

Lint free.

Test: bin/test -vvt 'external.*bug'

[Brad Crittenden (bac)]

Looks nice Gavin.

Since 'remote_version_dict' may be a dict or a tuple how about just calling it 'remote_version'?

review: Approve (code)

[Gavin Panella (allenap)]

Thanks Brad! Good catch; I've changed the name.

=== modified file 'lib/lp/bugs/doc/externalbugtracker-bugzilla.txt'
--- lib/lp/bugs/doc/externalbugtracker-bugzilla.txt	2009-09-23 12:24:10 +0000
+++ lib/lp/bugs/doc/externalbugtracker-bugzilla.txt	2010-01-13 10:28:18 +0000
@@ -24,11 +24,11 @@
 
     >>> from canonical.testing import LaunchpadZopelessLayer
     >>> txn = LaunchpadZopelessLayer.txn
-    >>> external_bugzilla = Bugzilla('http://example.com/', version='A.B4')
+    >>> external_bugzilla = Bugzilla('http://example.com/', version='A.B')
     Traceback (most recent call last):
       ...
     UnparseableBugTrackerVersion:
-    Failed to parse version 'A.B4' for http://...
+    Failed to parse version 'A.B' for http://...
 
 The version parsing is carried out by the Bugzilla._parseVersion()
 method, which takes a version string and returns a tuple of
@@ -51,7 +51,7 @@
 + characters in the version string will be removed.
 
     >>> print external_bugzilla._parseVersion('3.2+1')
-    (3, 21)
+    (3, 2, 1)
 
 Since we don't want to depend on a working network connection, we use a
 slightly modified implementation.
@@ -201,6 +201,31 @@
     >>> isinstance(bugzilla_to_use, BugzillaLPPlugin)
     True
 
+Older versions of the Bugzilla API return tuples rather than mappings
+in response to XML-RPC calls. When something other than a mapping is
+returned, the standard non-API non-plugin external bug tracker is
+selected.
+
+    >>> class OldXMLRPCTransport(xmlrpclib.Transport):
+    ...     def request(self, host, handler, request, verbose=None):
+    ...         args, method_name = xmlrpclib.loads(request)
+    ...
+    ...         if method_name == 'Bugzilla.version':
+    ...             return ('versionResponse', {'version': '3.2.5+'})
+    ...         else:
+    ...             raise xmlrpclib.Fault('Client', 'No such method')
+    ...
+    >>> test_transport = OldXMLRPCTransport()
+
+    >>> bugzilla._test_xmlrpc_proxy = xmlrpclib.ServerProxy(
+    ...     'http://example.com/xmlrpc.cgi',
+    ...     transport=test_transport)
+
+    >>> bugzilla_to_use = bugzilla.getExternalBugTrackerToUse()
+    >>> (isinstance(bugzilla_to_use, BugzillaAPI) or
+    ...  isinstance(bugzilla_to_use, BugzillaLPPlugin))
+    False
+
 
 == Status Conversion ==
 

=== modified file 'lib/lp/bugs/externalbugtracker/bugzilla.py'
--- lib/lp/bugs/externalbugtracker/bugzilla.py	2010-01-06 14:35:52 +0000
+++ lib/lp/bugs/externalbugtracker/bugzilla.py	2010-01-13 10:28:18 +0000
@@ -12,6 +12,7 @@
     ]
 
 import pytz
+import re
 import xml.parsers.expat
 import xmlrpclib
 
@@ -69,7 +70,7 @@
         try:
             # We try calling Bugzilla.version() on the remote
             # server because it's the most lightweight method there is.
-            remote_version_dict = proxy.Bugzilla.version()
+            remote_version = proxy.Bugzilla.version()
         except xmlrpclib.Fault, fault:
             if fault.faultCode == 'Client':
                 return False
@@ -88,10 +89,12 @@
             # The server returned an unparsable response.
             return False
         else:
-            if remote_version_dict['version'] >= '3.4':
-                return True
-            else:
-                return False
+            # Older versions of the Bugzilla API return tuples. We
+            # consider anything other than a mapping to be unsupported.
+            if isinstance(remote_version, dict):
+                if remote_version['version'] >= '3.4':
+                    return True
+            return False
 
     def _remoteSystemHasPluginAPI(self):
         """Return True if the remote host has the Launchpad plugin installed.
@@ -191,25 +194,13 @@
         if version is None:
             return None
 
-        try:
-            # XXX 2008-09-15 gmb bug 270695:
-            #     We can clean this up by just stripping out anything
-            #     not in [0-9\.].
-            # Get rid of trailing -rh, -debian, etc.
-            version = version.split("-")[0]
-            # Ignore plusses in the version.
-            version = version.replace("+", "")
-            # Ignore the 'rc' string in release candidate versions.
-            version = version.replace("rc", "")
-            # We need to convert the version to a tuple of integers if
-            # we are to compare it correctly.
-            version = tuple(int(x) for x in version.split("."))
-        except ValueError:
+        version_numbers = re.findall('[0-9]+', version)
+        if len(version_numbers) == 0:
             raise UnparseableBugTrackerVersion(
                 'Failed to parse version %r for %s' %
                 (version, self.baseurl))
 
-        return version
+        return tuple(int(number) for number in version_numbers)
 
     def convertRemoteImportance(self, remote_importance):
         """See `ExternalBugTracker`.