bugzilla: Remove custom boolean query format entirely

crobinso · crobinso · commit d4d288c48a6c · 2016-05-25T15:23:52.000-04:00
Warn if the old boolean properties appear to be used, but pass them
on to bugzilla and let it fail
diff --git a/bin/bugzilla b/bin/bugzilla
@@ -302,27 +302,26 @@ def _setup_action_query_parser(subparsers):
 
     # Boolean Charts
     bgrp = p.add_argument_group("Boolean options")
-    bgrp.add_argument('-B', '--booleantype', default="substring",
-            help="specify searching option for booleans, ie. substring, "
-                 "notsubstring, exact, ... [Default: substring]")
+    bgrp.add_argument('-B', '--booleantype',
+            help=argparse.SUPPRESS)
     bgrp.add_argument('--boolean_query', action="append",
             help=argparse.SUPPRESS)
-    bgrp.add_argument('--blocked', action="append",
+    bgrp.add_argument('--blocked',
             help="Search for bugs that block this bug ID")
-    bgrp.add_argument('--dependson', action="append",
+    bgrp.add_argument('--dependson',
             help="Search for bugs that depend on this bug ID")
-    bgrp.add_argument('--flag', action='append',
+    bgrp.add_argument('--flag',
             help="Search for bugs that have certain "
                  "flag states present. Ex --flags=dev_ack+")
-    bgrp.add_argument('--qa_whiteboard', action="append",
+    bgrp.add_argument('--qa_whiteboard',
             help="search for bugs that have certain QA "
                  "Whiteboard text present")
-    bgrp.add_argument('--devel_whiteboard', action="append",
+    bgrp.add_argument('--devel_whiteboard',
             help="search for bugs that have certain "
                  "Devel Whiteboard text present")
-    bgrp.add_argument('--alias', action="append",
+    bgrp.add_argument('--alias',
             help="search for bugs that have the provided alias")
-    bgrp.add_argument('--fixed_in', action="append",
+    bgrp.add_argument('--fixed_in',
             help="search Status Whiteboard field for specified words")
 
     _parser_add_output_options(p)
@@ -609,7 +608,7 @@ def _do_query(bz, opt, parser):
         priority=getattr(opt, "priority", None),
         target_milestone=getattr(opt, "target_milestone", None),
         emailtype=opt.emailtype or None,
-        booleantype=opt.booleantype,
+        booleantype=opt.booleantype or None,
         include_fields=include_fields,
         quicksearch=getattr(opt, "quicksearch", None),
         savedsearch=getattr(opt, "savedsearch", None),
diff --git a/bugzilla/base.py b/bugzilla/base.py
@@ -1050,24 +1050,11 @@ def build_query(self,
 
         Then pass the output to Bugzilla.query()
         """
-        if boolean_query:
+        if boolean_query or booleantype:
             raise RuntimeError("boolean_query format is no longer supported. "
                 "If you need complicated URL queries, look into "
                 "query --from-url/url_to_query().")
 
-        for key, val in [
-            ('fixed_in', fixed_in),
-            ('blocked', blocked),
-            ('dependson', dependson),
-            ('flag', flag),
-            ('qa_whiteboard', qa_whiteboard),
-            ('devel_whiteboard', devel_whiteboard),
-            ('alias', alias),
-        ]:
-            if val is not None:
-                raise RuntimeError("'%s' search not supported by this "
-                                   "bugzilla's XMLRPC interface" % key)
-
         query = {
             "product": self._listify(product),
             "component": self._listify(component),
@@ -1089,6 +1076,18 @@ def build_query(self,
             "quicksearch": quicksearch,
             "savedsearch": savedsearch,
             "sharer_id": savedsearch_sharer_id,
+
+            # RH extensions that we have to maintain here for back compat,
+            # but all future custom fields should be specified via
+            # cli --field option, or via extending the query dict() manually.
+            # No more supporting custom fields in this API
+            "cf_fixed_in": fixed_in,
+            "blocked": blocked,
+            "dependson": dependson,
+            "flagtypes.name": flag,
+            "cf_qa_whiteboard": qa_whiteboard,
+            "cf_devel_whiteboard": devel_whiteboard,
+            "alias": alias,
             "sub_components": self._listify(sub_component),
         }
 
diff --git a/bugzilla/rhbugzilla.py b/bugzilla/rhbugzilla.py
@@ -304,142 +304,42 @@ def post_translation(self, query, bug):
                     values += vallist
                 bug['sub_component'] = " ".join(values)
 
-    def build_external_tracker_boolean_query(
-            self, ext_type_description=None, ext_type_url=None,
-            ext_bz_bug_id=None, ext_status=None):
-        """
-        Helper method to build a boolean query to find bugs that contain an
-        external tracker.
-
-        All parameters that are None will be ignored when building the query.
-
-        ext_type_description: The external tracker description as used by
-            Bugzilla.
-        ext_type_url: The external tracker url as used by Bugzilla.
-        ext_bz_bug_id: The external bug id (ie: the bug number in the
-            external tracker).
-        ext_status: The status of the external bug.
-        """
-        parts = []
+    def build_external_tracker_boolean_query(self, *args, **kwargs):
+        ignore1 = args
+        ignore2 = kwargs
+        raise RuntimeError("Building external boolean queries is "
+            "no longer supported. Please build a URL query "
+            "via the bugzilla web UI and pass it to 'query --from-url' "
+            "or url_to_query()")
 
-        if ext_type_description is not None:
-            parts.append(
-                'external_bugzilla.description-equals-{0:s}'.format(
-                    ext_type_description))
-
-        if ext_type_url is not None:
-            parts.append(
-                'external_bugzilla.url-equals-{0:s}'.format(ext_type_url))
-
-        if ext_bz_bug_id is not None:
-            id_str = str(ext_bz_bug_id)
-            parts.append(
-                'ext_bz_bug_map.ext_bz_bug_id-equals-{0:s}'.format(id_str))
-
-        if ext_status is not None:
-            parts.append(
-                'ext_bz_bug_map.ext_status-equals-{0:s}'.format(ext_status))
-
-        return ' & '.join(parts)
 
     def build_query(self, **kwargs):
-        query = {}
-
-        def _add_key(paramname, keyname, listify=False):
-            val = kwargs.pop(paramname, None)
-            if val is None:
-                return
-            if listify:
-                val = self._listify(val)
-            query[keyname] = val
-
-        def bool_smart_split(boolval):
-            # This parses the CLI command syntax, but we only want to
-            # do space splitting if the space is actually part of a
-            # boolean operator
-            boolchars = ["|", "&", "!"]
-            add = ""
-            retlist = []
-
-            for word in boolval.split(" "):
-                if word.strip() in boolchars:
-                    word = word.strip()
-                    if add:
-                        retlist.append(add)
-                        add = ""
-                    retlist.append(word)
-                else:
-                    if add:
-                        add += " "
-                    add += word
-
-            if add:
-                retlist.append(add)
-            return retlist
-
-        def add_boolean(kwkey, key, bool_id):
-            value = self._listify(kwargs.pop(kwkey, None))
-            if value is None:
-                return bool_id
-
-            query["query_format"] = "advanced"
-            for boolval in value:
-                and_count = 0
-                or_count = 0
-
-                def make_bool_str(prefix):
-                    # pylint: disable=cell-var-from-loop
-                    return "%s%i-%i-%i" % (prefix, bool_id,
-                                           and_count, or_count)
-
-                for par in bool_smart_split(boolval):
-                    field = None
-                    fval = par
-                    typ = kwargs.get("booleantype", "substring")
-
-                    if par == "&":
-                        and_count += 1
-                    elif par == "|":
-                        or_count += 1
-                    elif par == "!":
-                        query['negate%i' % bool_id] = 1
-                    elif not key:
-                        if par.find('-') == -1:
-                            raise RuntimeError('Malformed boolean query: %s' %
-                                               value)
-
-                        args = par.split('-', 2)
-                        field = args[0]
-                        typ = args[1]
-                        fval = None
-                        if len(args) == 3:
-                            fval = args[2]
-                    else:
-                        field = key
-
-                    query[make_bool_str("field")] = field
-                    if fval:
-                        query[make_bool_str("value")] = fval
-                    query[make_bool_str("type")] = typ
-
-                bool_id += 1
-            return bool_id
-
-        # Use fancy email specification for RH bugzilla. It isn't
-        # strictly required, but is more powerful, and it is what
-        # bin/bugzilla historically generated. This requires
-        # query_format='advanced' which is an RHBZ only XMLRPC extension
-
-        chart_id = 0
-        chart_id = add_boolean("fixed_in", "cf_fixed_in", chart_id)
-        chart_id = add_boolean("blocked", "blocked", chart_id)
-        chart_id = add_boolean("dependson", "dependson", chart_id)
-        chart_id = add_boolean("flag", "flagtypes.name", chart_id)
-        chart_id = add_boolean("qa_whiteboard", "cf_qa_whiteboard", chart_id)
-        chart_id = add_boolean("devel_whiteboard", "cf_devel_whiteboard",
-                               chart_id)
-        chart_id = add_boolean("alias", "alias", chart_id)
+        # We previously accepted a text format to approximate boolean
+        # queries, and only for RHBugzilla. Upstream bz has --from-url
+        # support now, so point people to that instead so we don't have
+        # to document and maintain this logic anymore
+        def _warn_bool(kwkey):
+            vallist = self._listify(kwargs.get(kwkey, None))
+            for value in vallist or []:
+                for s in value.split(" "):
+                    if s not in ["|", "&", "!"]:
+                        continue
+                    log.warn("%s value '%s' appears to use the now "
+                        "unsupported boolean formatting, your query may "
+                        "be incorrect. If you need complicated URL queries, "
+                        "look into bugzilla --from-url/url_to_query().",
+                        kwkey, value)
+                    return
+
+        _warn_bool("fixed_in")
+        _warn_bool("blocked")
+        _warn_bool("dependson")
+        _warn_bool("flag")
+        _warn_bool("qa_whiteboard")
+        _warn_bool("devel_whiteboard")
+        _warn_bool("alias")
 
+        query = {}
         query.update(self._process_include_fields(None, None,
             kwargs.pop('extra_fields', None)))
 
diff --git a/tests/query.py b/tests/query.py
@@ -91,13 +91,6 @@ def testKeywords(self):
                     "--url http://example.com --url_type foo",
                     self._keywords_out)
 
-    def testBooleans(self):
-        self.clicomm("--blocked 123456 "
-                    "--devel_whiteboard 'foobar | baz' "
-                    "--qa_whiteboard '! baz foo' "
-                    "--flag 'needinfo & devel_ack'",
-                    self._booleans_out)
-
     def testBooleanChart(self):
         self.clicomm("--boolean_query 'keywords-substring-Partner & "
                     "keywords-notsubstring-OtherQA' "
@@ -151,7 +144,6 @@ def testSubComponent(self):
     _components_file_out = {'component': ["foo", "bar", "baz"]}
     _keywords_out = {'keywords': 'Triaged', 'bug_file_loc':
         'http://example.com', 'bug_file_loc_type': 'foo'}
-    _booleans_out = None
     _longdesc_out = {'longdesc': 'foobar', 'longdesc_type': 'allwordssubstr',
         'query_format': 'advanced'}
     _quicksearch_out = {'quicksearch': 'foo bar baz'}
@@ -224,18 +216,7 @@ class RHBZTest(BZ4Test):
     _output_format_out = BZ34Test.output_format_out.copy()
     _output_format_out["include_fields"] = ['product', 'summary',
         'platform', 'status', 'id', 'blocks', 'whiteboard']
-    _booleans_out = {'value2-0-0': 'baz foo', 'value0-0-0': '123456',
-        'type3-0-1': 'substring', 'value1-1-0': 'devel_ack', 'type0-0-0':
-        'substring', 'type2-0-0': 'substring', 'field3-0-1':
-        'cf_devel_whiteboard', 'field3-0-0': 'cf_devel_whiteboard',
-        'field1-0-0': 'flagtypes.name', 'value3-0-0': 'foobar',
-        'value3-0-1': 'baz', 'value1-0-0': 'needinfo', 'type1-1-0':
-        'substring', 'type1-0-0': 'substring', 'field1-1-0':
-        'flagtypes.name', 'negate2': 1, 'field2-0-0':
-        'cf_qa_whiteboard', 'type3-0-0': 'substring', 'field0-0-0':
-        'blocked', 'include_fields': BZ4Test._default_includes,
-        'query_format': 'advanced'}
-
+    _booleans_out = {}
 
     def testTranslation(self):
         def translate(_in):
@@ -270,6 +251,28 @@ def testInvalidBoolean(self):
         self.assertRaises(RuntimeError, self.bz.build_query,
             boolean_query="foobar")
 
+    def testBooleans(self):
+        out = {
+            'blocked': '123456',
+            'cf_devel_whiteboard': 'foobar | baz',
+            'cf_qa_whiteboard': '! baz foo',
+            'flagtypes.name': 'needinfo & devel_ack',
+            'include_fields': ['assigned_to', 'id', 'status', 'summary']
+        }
+
+        import bugzilla
+        import logging
+        log = logging.getLogger(bugzilla.__name__)
+        handlers = log.handlers
+        try:
+            log.handlers = []
+            self.clicomm("--blocked 123456 "
+                        "--devel_whiteboard 'foobar | baz' "
+                        "--qa_whiteboard '! baz foo' "
+                        "--flag 'needinfo & devel_ack'", out)
+        finally:
+            log.handlers = handlers
+
 
 class TestURLToQuery(BZ34Test):
     def _check(self, url, query):
diff --git a/tests/rw_functional.py b/tests/rw_functional.py
