rhbugzilla: Fix include_fields=["fixed_in"] (bz 1084887)

crobinso · crobinso · commit 5d2f7a848ae1 · 2014-04-14T12:13:50.000-04:00
We want bug.fixed_in to map to bug.cf_fixed_in, but we _don't_ want to
do that mapping for include_fields, since cf_fixed_in is the actual API
name.

Differentiate field_aliases by whether they are bug attribute or API aliases.
diff --git a/bugzilla/base.py b/bugzilla/base.py
@@ -196,6 +196,28 @@ class BugzillaError(Exception):
     pass
 
 
+class _FieldAlias(object):
+    """
+    Track API attribute names that differ from what we expose in users.
+
+    For example, originally 'short_desc' was the name of the property that
+    maps to 'summary' on modern bugzilla. We want pre-existing API users
+    to be able to continue to use Bug.short_desc, and
+    query({"short_desc": "foo"}). This class tracks that mapping.
+
+    @oldname: The old attribute name
+    @newname: The modern attribute name
+    @is_api: If True, use this mapping for values sent to the xmlrpc API
+        (like the query example)
+    @is_bug: If True, use this mapping for Bug attribute names.
+    """
+    def __init__(self, newname, oldname, is_api=True, is_bug=True):
+        self.newname = newname
+        self.oldname = oldname
+        self.is_api = is_api
+        self.is_bug = is_bug
+
+
 class BugzillaBase(object):
     '''An object which represents the data and methods exported by a Bugzilla
     instance. Uses xmlrpclib to do its thing. You'll want to create one thusly:
@@ -309,6 +331,29 @@ def __init__(self, url=None, user=None, password=None, cookiefile=-1,
             cookiefile = os.path.expanduser('~/.bugzillacookies')
         self.cookiefile = cookiefile
 
+        # List of field aliases. Maps old style RHBZ parameter
+        # names to actual upstream values. Used for createbug() and
+        # query include_fields at least.
+        self._field_aliases = []
+        self._add_field_alias('summary', 'short_desc')
+        self._add_field_alias('description', 'comment')
+        self._add_field_alias('platform', 'rep_platform')
+        self._add_field_alias('severity', 'bug_severity')
+        self._add_field_alias('status', 'bug_status')
+        self._add_field_alias('id', 'bug_id')
+        self._add_field_alias('blocks', 'blockedby')
+        self._add_field_alias('blocks', 'blocked')
+        self._add_field_alias('depends_on', 'dependson')
+        self._add_field_alias('creator', 'reporter')
+        self._add_field_alias('url', 'bug_file_loc')
+        self._add_field_alias('dupe_of', 'dupe_id')
+        self._add_field_alias('dupe_of', 'dup_id')
+        self._add_field_alias('comments', 'longdescs')
+        self._add_field_alias('creation_time', 'opendate')
+        self._add_field_alias('creation_time', 'creation_ts')
+        self._add_field_alias('whiteboard', 'status_whiteboard')
+        self._add_field_alias('last_change_time', 'delta_ts')
+
         if url:
             self.connect(url)
 
@@ -362,6 +407,17 @@ def _product_name_to_id(self, product):
                 return p['id']
         raise ValueError('No product named "%s"' % product)
 
+    def _add_field_alias(self, *args, **kwargs):
+        self._field_aliases.append(_FieldAlias(*args, **kwargs))
+
+    def _get_bug_aliases(self):
+        return [(f.newname, f.oldname)
+                for f in self._field_aliases if f.is_bug]
+
+    def _get_api_aliases(self):
+        return [(f.newname, f.oldname)
+                for f in self._field_aliases if f.is_api]
+
 
     ###################
     # Cookie handling #
@@ -763,31 +819,6 @@ def _find_comps():
     # like this, for upstream bz it returns all info for every Bug.get()
     getbug_extra_fields = []
 
-    # List of field aliases. Maps old style RHBZ parameter names to actual
-    # upstream values. Used for createbug() and query include_fields at
-    # least.
-    #
-    # Format is (currentname, oldname)
-    field_aliases = (
-        ('summary', 'short_desc'),
-        ('description', 'comment'),
-        ('platform', 'rep_platform'),
-        ('severity', 'bug_severity'),
-        ('status', 'bug_status'),
-        ('id', 'bug_id'),
-        ('blocks', 'blockedby'),
-        ('blocks', 'blocked'),
-        ('depends_on', 'dependson'),
-        ('creator', 'reporter'),
-        ('url', 'bug_file_loc'),
-        ('dupe_of', 'dupe_id'),
-        ('dupe_of', 'dup_id'),
-        ('comments', 'longdescs'),
-        ('creation_time', 'opendate'),
-        ('creation_time', 'creation_ts'),
-        ('whiteboard', 'status_whiteboard'),
-        ('last_change_time', 'delta_ts'),
-    )
 
     def _getbugs(self, idlist, simple=False, permissive=True):
         '''
@@ -1432,11 +1463,11 @@ def _validate_createbug(self, *args, **kwargs):
 
         # If we're getting a call that uses an old fieldname, convert it to the
         # new fieldname instead.
-        for newfield, oldfield in self.field_aliases:
-            if (newfield in self.createbug_required and
-                newfield not in data and
-                oldfield in data):
-                data[newfield] = data.pop(oldfield)
+        for newname, oldname in self._get_api_aliases():
+            if (newname in self.createbug_required and
+                newname not in data and
+                oldname in data):
+                data[newname] = data.pop(oldname)
 
         # Back compat handling for check_args
         if "check_args" in data:
diff --git a/bugzilla/bug.py b/bugzilla/bug.py
@@ -72,7 +72,7 @@ def __getattr__(self, name):
                 return self.__dict__[name]
 
             # Check field aliases
-            for newname, oldname in self.bugzilla.field_aliases:
+            for newname, oldname in self.bugzilla._get_bug_aliases():
                 if name == oldname and newname in self.__dict__:
                     return self.__dict__[newname]
 
@@ -123,7 +123,7 @@ def _update_dict(self, newdict):
         '''
         self.bugzilla.post_translation({}, newdict)
 
-        for newname, oldname in self.bugzilla.field_aliases:
+        for newname, oldname in self.bugzilla._get_bug_aliases():
             if not oldname in newdict:
                 continue
 
diff --git a/bugzilla/bugzilla4.py b/bugzilla/bugzilla4.py
@@ -33,11 +33,8 @@ def build_query(self, **kwargs):
 
         # 'include_fields' only available for Bugzilla4+
         include_fields = kwargs.get('include_fields', None)
-        if not include_fields is None:
-            query["include_fields"] = include_fields
-
-            # Translate old style fields
-            for newname, oldname in self.field_aliases:
+        if include_fields is not None:
+            for newname, oldname in self._get_api_aliases():
                 if oldname in include_fields:
                     include_fields.remove(oldname)
                     if newname not in include_fields:
@@ -46,6 +43,7 @@ def build_query(self, **kwargs):
             # We always need the id
             if 'id' not in include_fields:
                 include_fields.append('id')
+            query["include_fields"] = include_fields
 
         return query
 
diff --git a/bugzilla/rhbugzilla.py b/bugzilla/rhbugzilla.py
@@ -51,6 +51,23 @@ def __init__(self, **kwargs):
 
         _parent.__init__(self, **kwargs)
 
+        def _add_both_alias(newname, origname):
+            self._add_field_alias(newname, origname, is_api=False)
+            self._add_field_alias(origname, newname, is_bug=False)
+
+        _add_both_alias('fixed_in', 'cf_fixed_in')
+        _add_both_alias('qa_whiteboard', 'cf_qa_whiteboard')
+        _add_both_alias('devel_whiteboard', 'cf_devel_whiteboard')
+        _add_both_alias('internal_whiteboard', 'cf_internal_whiteboard')
+
+        self._add_field_alias('component', 'components', is_bug=False)
+        self._add_field_alias('version', 'versions', is_bug=False)
+        self._add_field_alias('sub_component', 'sub_components', is_bug=False)
+
+        # flags format isn't exactly the same but it's the closest approx
+        self._add_field_alias('flags', 'flag_types')
+
+
     getbug_extra_fields = (
         _parent.getbug_extra_fields + [
             "attachments", "comments", "description",
@@ -59,17 +76,6 @@ def __init__(self, **kwargs):
         ]
     )
 
-    field_aliases = (
-        _parent.field_aliases + (
-            ('fixed_in', 'cf_fixed_in'),
-            ('qa_whiteboard', 'cf_qa_whiteboard'),
-            ('devel_whiteboard', 'cf_devel_whiteboard'),
-            ('internal_whiteboard', 'cf_internal_whiteboard'),
-            # Format isn't exactly the same but it's the closest approximation
-            ('flags', 'flag_types'),
-        )
-    )
-
 
     ######################
     # Bug update methods #
@@ -138,14 +144,8 @@ def pre_translation(self, query):
                 query['include_fields'] = query['column_list']
                 del query['column_list']
 
-        include_aliases = (
-            ("component", "components"),
-            ("version", "versions"),
-            ("sub_components", "sub_component"),
-        )
-
         include_fields = query['include_fields']
-        for newname, oldname in (self.field_aliases + include_aliases):
+        for newname, oldname in self._get_api_aliases():
             if oldname in include_fields:
                 include_fields.remove(oldname)
                 if newname not in include_fields:
diff --git a/tests/bug.py b/tests/bug.py
@@ -14,8 +14,6 @@
 from bugzilla import RHBugzilla
 from bugzilla.bug import _Bug
 
-import tests
-
 
 rhbz = RHBugzilla(cookiefile=None)
 
diff --git a/tests/query.py b/tests/query.py
@@ -9,6 +9,7 @@
 Unit tests for building query strings with bin/bugzilla
 '''
 
+import copy
 import os
 import unittest
 
@@ -242,6 +243,21 @@ class RHBZTest(BZ4Test):
         'component': ["lvm2", "kernel"],
         'sub_components': ["Command-line tools (RHEL5)"]}
 
+    def testTranslation(self):
+        in_query = {
+            "fixed_in": "foo.bar",
+            "product": "some-product",
+            "cf_devel_whiteboard": "some_devel_whiteboard",
+            "include_fields": ["fixed_in",
+                "components", "cf_devel_whiteboard"],
+        }
+        out_query = copy.deepcopy(in_query)
+        self.bz.pre_translation(out_query)
+
+        in_query["include_fields"] = [
+            "cf_devel_whiteboard", "cf_fixed_in", "component"]
+        self.assertDictEqual(in_query, out_query)
+
 
 class TestURLToQuery(BZ34Test):
     def _check(self, url, query):
