File bugs via XML-RPC when possible

If the server supports XML-RPC (3.0 or newer), then filing via
xmlrpc.cgi has advantages:

 - We get a string error message instead of an HTML page
 - We can not specify priority/severity and let the server
   default it; this is important since there aren't standard
   priorities.

So, we try to first file via xmlrpc.cgi's Bug.create method and
if we get a 404, fall back to the old method of a form post.

The configuration for a couple of fields is changed to use the human
readable field names from Bug.create rather than the database-field
names of post_bug.cgi.

 default-bug-severity => default-severity
 default-rep-platform => default-platform

2 files changed, 89 insertions, 20 deletions

diff --git a/TODO b/TODO
index 27197b1..5629e56 100644
--- a/TODO
+++ b/TODO
@@ -24,23 +24,15 @@ Automatically guess obvious obsoletes
  matches the Subject of the attachment, start the Obsoletes line
  uncommented?
 
-Use XML-RPC when available.
-
-  Maybe use python-bugzilla: http://fedorahosted.org/python-bugzilla/
-
-  Not sure there are a lot of advantages to this; one thing that it
-  might be possible to do with this is allow the user to specify only
-  the product and get an interactive list of components. Also, better
-  error handling.
-
 Handle redirects:
 
   Should follow redirects, both to different URLs and http => https
 
 Better display of errors
 
-  Currently specifying a non-existent product/component just dumps
-  out raw HTML for the reply. Etc.
+  The switch to XML-RPC greatly improves errors when filing a new bug,
+  but other problems (e.g., having stale login cookies when making an
+  attachment) still just dump HTML pages error pages to the console.
 
 More general patch application
 
diff --git a/git-bz b/git-bz
index 62ca8ae..92bdd50 100755
--- a/git-bz
+++ b/git-bz
@@ -154,11 +154,11 @@
 #
 # 1) git configuration variables specified for the alias.
 #
-#    git config --global bz-tracker.gnome.default-bug-severity trivial
+#    git config --global bz-tracker.gnome.default-severity trivial
 #
 # 2) git configuration variables specified for the host
 #
-#    git config --global bz-tracker.bugzilla.gnome.org.default-bug-severity trivial
+#    git config --global bz-tracker.bugzilla.gnome.org.default-severity trivial
 #
 # 3) Host specific configuration in this file, see the CONFIG variable below
 #
@@ -170,11 +170,8 @@
 DEFAULT_CONFIG = \
 """
 default-assigned-to =
-default-bug-file-loc =
-default-bug-severity = normal
 default-op-sys = All
-default-priority = P5
-default-rep-platform = All
+default-platform = All
 default-version = unspecified
 """
 
@@ -213,6 +210,7 @@ import sys
 import tempfile
 import time
 import traceback
+import xmlrpclib
 import urllib
 from xml.etree.cElementTree import ElementTree
 
@@ -726,6 +724,9 @@ class BugPatch(object):
         self.date = date
         self.data = data
 
+class NoXmlRpcError(Exception):
+    pass
+
 class BugServer(object):
     def __init__(self, host, https):
         self.host = host
@@ -734,9 +735,10 @@ class BugServer(object):
         self.cookies = get_bugzilla_cookies(host)
 
         self._connection = None
+        self._xmlrpc_proxy = None
 
     def get_connection(self):
-        if not self._connection:
+        if self._connection is None:
             if self.https:
                 self._connection = httplib.HTTPSConnection(self.host, 443)
             else:
@@ -760,6 +762,32 @@ class BugServer(object):
         content_type, body = encode_multipart_formdata(fields, files)
         return self.send_request("POST", url, data=body, headers={ 'Content-Type': content_type })
 
+    def get_xmlrpc_proxy(self):
+        if self._xmlrpc_proxy is None:
+            uri = "%s://%s/xmlrpc.cgi" % ("https" if self.https else "http",
+                                          self.host)
+            transport = BugTransport(self)
+            self._xmlrpc_proxy = xmlrpclib.ServerProxy(uri, transport)
+
+        return self._xmlrpc_proxy
+
+class BugTransport(xmlrpclib.Transport):
+    def __init__(self, server):
+        xmlrpclib.Transport.__init__(self)
+        self.server = server
+
+    # Overriding this allows us not to separately subclass Transport and SafeTransport
+    def make_connection(self, host):
+        if self.server.https:
+            return httplib.HTTPS(self.server.host)
+        else:
+            return httplib.HTTP(self.server.host)
+
+    # This is the main point of the subclassing - to add cookies
+    def send_request(self, connection, *args):
+        xmlrpclib.Transport.send_request(self, connection, *args)
+        connection.putheader("Cookie", self.server.get_cookie_string())
+
 class Bug(object):
     def __init__(self, server):
         self.server = server
@@ -803,13 +831,56 @@ class Bug(object):
 
                 self.patches.append(BugPatch(attach_id, description, date, data))
 
-    def _create(self, product, component, short_desc, comment, default_fields):
-        fields = dict(default_fields)
+    def _create_via_xmlrpc(self, product, component, short_desc, comment, default_fields):
+        params = dict()
+        params['product'] = product
+        params['component'] = component
+        params['summary'] = short_desc
+        params['description'] = comment
+        for (field, value) in default_fields.iteritems():
+            params[field] = value
+
+        try:
+            response = self.server.get_xmlrpc_proxy().Bug.create(params)
+            self.id = response['id']
+        except xmlrpclib.Fault, e:
+            die(e.faultString)
+        except xmlrpclib.ProtocolError, e:
+            if e.errcode == 404:
+                raise NoXmlRpcError(e.errmsg)
+            else:
+                die("Problem communicating with server: %s (%d)", e.errmsg, e.errcode)
+
+    def _create_with_form(self, product, component, short_desc, comment, default_fields):
+        fields = dict()
         fields['product'] = product
         fields['component'] = component
         fields['short_desc'] = short_desc
         fields['comment'] = comment
 
+        # post_bug.cgi wants some names that are less congenial than the names
+        # expected in XML-RPC.
+        for (field, value) in default_fields.iteritems():
+            if field == 'severity':
+                field = 'bug_severity'
+            elif field == 'platform':
+                field = 'rep_platform'
+            fields[field] = value
+
+        # Priority values vary wildly between different servers because the stock
+        # Bugzilla uses the awkward P1/../P5. It will be defaulted on the XML-RPC
+        # code path, but we need to take a wild guess here.
+        if not 'priority' in fields:
+            fields['priority'] = 'P5'
+        # Legal severity values are much more standardized, but not specifying it
+        # in the XML-RPC code path allows the server default to win. We need to
+        # specify something here.
+        if not 'severity' in fields:
+            fields['bug_severity'] = 'normal'
+        # Required, but a configured default doesn't make any sense
+        if not 'bug_file_loc' in fields:
+            fields['bug_file_loc'] = ''
+
         files = {}
 
         response = self.server.send_post("/post_bug.cgi", fields, files)
@@ -826,6 +897,12 @@ class Bug(object):
 
         self.id = int(m.group(1))
 
+    def _create(self, product, component, short_desc, comment, default_fields):
+        try:
+            self._create_via_xmlrpc(product, component, short_desc, comment, default_fields)
+        except NoXmlRpcError:
+            self._create_with_form(product, component, short_desc, comment, default_fields)
+
         print "Successfully created"
         print "Bug %d - %s" % (self.id, short_desc)
         print self.get_url()