rhbugzilla: Turn converter functions into a static class

crobinso · crobinso · commit 1bc70c4e17ef · 2020-01-10T14:39:15.000-05:00
This decouples the class inheritance magic and makes it more clear
in Bugzilla() when exactly behavior is changing for rhbz handling

Signed-off-by: Cole Robinson &lt;crobinso@redhat.com&gt;
diff --git a/bugzilla/__init__.py b/bugzilla/__init__.py
@@ -9,10 +9,9 @@
 from .apiversion import version, __version__
 from .base import Bugzilla
 from .exceptions import BugzillaError
-from .rhbugzilla import RHBugzilla
 from .oldclasses import (Bugzilla3, Bugzilla32, Bugzilla34, Bugzilla36,
         Bugzilla4, Bugzilla42, Bugzilla44,
-        NovellBugzilla, RHBugzilla3, RHBugzilla4)
+        NovellBugzilla, RHBugzilla, RHBugzilla3, RHBugzilla4)
 
 
 # This is the public API. If you are explicitly instantiating any other
diff --git a/bugzilla/base.py b/bugzilla/base.py
@@ -23,6 +23,7 @@
 from ._compatimports import Mapping, urlparse, urlunparse, parse_qsl
 from .bug import Bug, User
 from .exceptions import BugzillaError
+from .rhbugzilla import _RHBugzillaConverters
 from ._session import _BugzillaSession
 from ._util import listify
 
@@ -233,33 +234,34 @@ def __init__(self, url=-1, user=None, password=None, cookiefile=-1,
         if url:
             self.connect(url)
 
+    def _detect_is_redhat_bugzilla(self):
+        if self._is_redhat_bugzilla:
+            return True
+
+        if "bugzilla.redhat.com" in self.url:
+            log.info("Using RHBugzilla for URL containing bugzilla.redhat.com")
+            return True
+
+        try:
+            extensions = self._backend.bugzilla_extensions()
+            if "RedHat" in extensions.get('extensions', {}):
+                log.info("Found RedHat bugzilla extension, "
+                    "using RHBugzilla")
+                return True
+        except Exception:
+            log.debug("Failed to fetch bugzilla extensions", exc_info=True)
+
+        return False
+
     def _init_class_from_url(self):
         """
         Detect if we should use RHBugzilla class, and if so, set it
         """
-        from .rhbugzilla import RHBugzilla  # pylint: disable=cyclic-import
-        if isinstance(self, RHBugzilla):
-            return
-
-        c = None
-        if "bugzilla.redhat.com" in self.url:
-            log.info("Using RHBugzilla for URL containing bugzilla.redhat.com")
-            c = RHBugzilla
-        else:
-            try:
-                extensions = self._backend.bugzilla_extensions()
-                if "RedHat" in extensions.get('extensions', {}):
-                    log.info("Found RedHat bugzilla extension, "
-                        "using RHBugzilla")
-                    c = RHBugzilla
-            except Exception:
-                log.debug("Failed to fetch bugzilla extensions", exc_info=True)
-
-        if not c:
-            return
+        from .oldclasses import RHBugzilla  # pylint: disable=cyclic-import
 
-        self.__class__ = c
-        self._is_redhat_bugzilla = True
+        if self._detect_is_redhat_bugzilla():
+            self.__class__ = RHBugzilla
+            self._is_redhat_bugzilla = True
 
     def _get_field_aliases(self):
         # List of field aliases. Maps old style RHBZ parameter
@@ -1279,12 +1281,18 @@ def pre_translation(self, query):
         In order to keep the API the same, Bugzilla4 needs to process the
         query and the result. This also applies to the refresh() function
         """
+        if self._is_redhat_bugzilla:
+            _RHBugzillaConverters.pre_translation(query)
+            query.update(self._process_include_fields(
+                query["include_fields"], None, None))
 
     def post_translation(self, query, bug):
         """
         In order to keep the API the same, Bugzilla4 needs to process the
         query and the result. This also applies to the refresh() function
         """
+        if self._is_redhat_bugzilla:
+            _RHBugzillaConverters.post_translation(query, bug)
 
     def bugs_history_raw(self, bug_ids):
         """
@@ -1396,18 +1404,28 @@ def build_update(self,
         https://bugzilla.readthedocs.io/en/latest/api/core/v1/bug.html#create-bug
         """
         ret = {}
+        rhbzret = {}
 
         # These are only supported for rhbugzilla
-        for key, val in [
-            ("fixed_in", fixed_in),
-            ("devel_whiteboard", devel_whiteboard),
-            ("qa_whiteboard", qa_whiteboard),
-            ("internal_whiteboard", internal_whiteboard),
-            ("sub_component", sub_component),
-        ]:
-            if val is not None:
-                raise ValueError("bugzilla instance does not support "
-                                 "updating '%s'" % key)
+        #
+        # This should not be extended any more.
+        # If people want to handle custom fields, manually extend the
+        # returned dictionary.
+        rhbzargs = {
+            "fixed_in": fixed_in,
+            "devel_whiteboard": devel_whiteboard,
+            "qa_whiteboard": qa_whiteboard,
+            "internal_whiteboard": internal_whiteboard,
+            "sub_component": sub_component,
+        }
+        if self._is_redhat_bugzilla:
+            rhbzret = _RHBugzillaConverters.convert_build_update(
+                component=component, **rhbzargs)
+        else:
+            for key, val in rhbzargs.items():
+                if val is not None:
+                    raise ValueError("bugzilla instance does not support "
+                                     "updating '%s'" % key)
 
         def s(key, val, convert=None):
             if val is None:
@@ -1479,6 +1497,7 @@ def c(val):
             if comment_private:
                 ret["comment"]["is_private"] = comment_private
 
+        ret.update(rhbzret)
         return ret
 
 
diff --git a/bugzilla/oldclasses.py b/bugzilla/oldclasses.py
@@ -2,7 +2,6 @@
 # See the COPYING file in the top-level directory.
 
 from .base import Bugzilla
-from .rhbugzilla import RHBugzilla
 
 # These are old compat classes. Nothing new should be added here,
 # and these should not be altered
@@ -40,6 +39,23 @@ class NovellBugzilla(Bugzilla):
     pass
 
 
+class RHBugzilla(Bugzilla):
+    """
+    Helper class for historical bugzilla.redhat.com back compat
+
+    Historically this class used many more non-upstream methods, but
+    in 2012 RH started dropping most of its custom bits. By that time,
+    upstream BZ had most of the important functionality.
+
+    Much of the remaining code here is just trying to keep things operating
+    in python-bugzilla back compatible manner.
+
+    This class was written using bugzilla.redhat.com's API docs:
+    https://bugzilla.redhat.com/docs/en/html/api/
+    """
+    _is_redhat_bugzilla = True
+
+
 class RHBugzilla3(RHBugzilla):
     pass
 
diff --git a/bugzilla/rhbugzilla.py b/bugzilla/rhbugzilla.py
@@ -8,56 +8,26 @@
 
 from logging import getLogger
 
-from .base import Bugzilla
 from ._util import listify
 
 log = getLogger(__name__)
 
 
-class RHBugzilla(Bugzilla):
+class _RHBugzillaConverters(object):
     """
-    Bugzilla class for connecting Red Hat's forked bugzilla instance,
-    bugzilla.redhat.com
-
-    Historically this class used many more non-upstream methods, but
-    in 2012 RH started dropping most of its custom bits. By that time,
-    upstream BZ had most of the important functionality.
-
-    Much of the remaining code here is just trying to keep things operating
-    in python-bugzilla back compatible manner.
-
-    This class was written using bugzilla.redhat.com's API docs:
-    https://bugzilla.redhat.com/docs/en/html/api/
+    Static class that holds functional Red Hat back compat converters.
+    Called inline in Bugzilla
     """
-    _is_redhat_bugzilla = True
-
-    ######################
-    # Bug update methods #
-    ######################
-
-    def build_update(self, **kwargs):
-        # pylint: disable=arguments-differ
+    @staticmethod
+    def convert_build_update(
+            component=None,
+            fixed_in=None,
+            qa_whiteboard=None,
+            devel_whiteboard=None,
+            internal_whiteboard=None,
+            sub_component=None):
         adddict = {}
 
-        def pop(key, destkey):
-            val = kwargs.pop(key, None)
-            if val is None:
-                return
-            adddict[destkey] = val
-
-        def get_sub_component():
-            val = kwargs.pop("sub_component", None)
-            if val is None:
-                return
-
-            if not isinstance(val, dict):
-                component = listify(kwargs.get("component"))
-                if not component:
-                    raise ValueError("component must be specified if "
-                        "specifying sub_component")
-                val = {component[0]: val}
-            adddict["sub_components"] = val
-
         def get_alias():
             # RHBZ has a custom extension to allow a bug to have multiple
             # aliases, so the format of aliases is
@@ -74,25 +44,35 @@ def get_alias():
             # Implementation will go here when it's available
             pass
 
-        pop("fixed_in", "cf_fixed_in")
-        pop("qa_whiteboard", "cf_qa_whiteboard")
-        pop("devel_whiteboard", "cf_devel_whiteboard")
-        pop("internal_whiteboard", "cf_internal_whiteboard")
+        if fixed_in is not None:
+            adddict["cf_fixed_in"] = fixed_in
+        if qa_whiteboard is not None:
+            adddict["cf_qa_whiteboard"] = qa_whiteboard
+        if devel_whiteboard is not None:
+            adddict["cf_devel_whiteboard"] = devel_whiteboard
+        if internal_whiteboard is not None:
+            adddict["cf_internal_whiteboard"] = internal_whiteboard
+
+        if sub_component:
+            if not isinstance(sub_component, dict):
+                component = listify(component)
+                if not component:
+                    raise ValueError("component must be specified if "
+                        "specifying sub_component")
+                sub_component = {component[0]: sub_component}
+            adddict["sub_components"] = sub_component
 
-        get_sub_component()
         get_alias()
 
-        vals = Bugzilla.build_update(self, **kwargs)
-        vals.update(adddict)
-
-        return vals
+        return adddict
 
 
     #################
     # Query methods #
     #################
 
-    def pre_translation(self, query):
+    @staticmethod
+    def pre_translation(query):
         """
         Translates the query for possible aliases
         """
@@ -118,15 +98,11 @@ def pre_translation(self, query):
                 query['include_fields'] = query['column_list']
                 del query['column_list']
 
-        # We need to do this for users here for users that
-        # don't call build_query
-        query.update(self._process_include_fields(query["include_fields"],
-            None, None))
-
         if old != query:
             log.debug("RHBugzilla pretranslated query to: %s", query)
 
-    def post_translation(self, query, bug):
+    @staticmethod
+    def post_translation(query, bug):
         """
         Convert the results of getbug back to the ancient RHBZ value
         formats
