and/or query combiner for filter options

Provides an option to specify query combiner ("and/or")
for attribute based filters in query parameter.
So if user wants to search artefact a based on filters :
field1 = value1 and (field2 = value2 or field3 = value3)

it can be specified in query like :
field1=eq:value1&field2=or:eq:value2&field3=or:eq:value3

"and" is the default value of combiner in case it is not specified.

Change-Id: I16c7c7fecefab5d615fa9e933234725249c5d97a
Implements: blueprint glare-search-artifact-query-update

glare/common/utils.py

@@ -343,32 +343,52 @@ def stash_conf_values():
 
 
 def split_filter_op(expression):
-    """Split operator from threshold in an expression.
-    Designed for use on a comparative-filtering query field.
-    When no operator is found, default to an equality comparison.
 
-    :param expression: the expression to parse
-    :return: a tuple (operator, threshold) parsed from expression
-    """
-    left, sep, right = expression.partition(':')
-    if sep:
-        # If the expression is a date of the format ISO 8601 like
-        # CCYY-MM-DDThh:mm:ss+hh:mm and has no operator, it should
-        # not be partitioned, and a default operator of eq should be
-        # assumed.
+    default_operator = "eq"
+    default_condition_combiner = "and"
+    combiner_options = ('and', 'or')
+
+    # all method return data in requence combiner, Operator, value
+    def one(expression):
+        return default_condition_combiner, default_operator, expression
+
+    def two(expression):
+        args = expression.split(":")
+        if args[0] in combiner_options:     # and:5, and:or, or:lte
+            return args[0], default_operator, args[1]
+        else:
+            return default_condition_combiner, args[0], args[1]
+
+    def multiple(expression):
+        args = expression.split(":")
+        combiner = default_condition_combiner
+        progress_index = 0
+
+        if args[0] in combiner_options:
+            combiner = args[0]
+            progress_index += 1
+
+        if _is_iso_date(":".join(args[progress_index:])):
+            expression = ":".join(args[progress_index:])
+            operator = default_operator
+        else:
+            operator = args[progress_index]
+            progress_index += 1
+            expression = ":".join(args[progress_index:])
+
+        return combiner, operator, expression
+
+    def _is_iso_date(arg):
         try:
-            timeutils.parse_isotime(expression)
-            op = 'eq'
-            threshold = expression
+            timeutils.parse_isotime(arg)
+            return True
         except ValueError:
-            op = left
-            threshold = right
-    else:
-        op = 'eq'  # default operator
-        threshold = left
+            return False
 
-    # NOTE stevelle decoding escaped values may be needed later
-    return op, threshold
+    switcher = {1: one, 2: two}
+    func = switcher.get(len(expression.split(":")), multiple)
+
+    return func(expression)
 
 
 def validate_quotes(value):

glare/db/sqlalchemy/api.py

@@ -26,7 +26,7 @@ import osprofiler.sqlalchemy
 from retrying import retry
 import six
 import sqlalchemy
-from sqlalchemy import and_
+from sqlalchemy import and_, distinct
 import sqlalchemy.exc
 from sqlalchemy import exists
 from sqlalchemy import func
@@ -254,9 +254,13 @@ def _apply_latest_filter(context, session, query,
 
 def _apply_user_filters(query, basic_conds, tag_conds, prop_conds):
 
+    or_queries = []
     if basic_conds:
-        for basic_condition in basic_conds:
+        for basic_condition in basic_conds['and']:
             query = query.filter(and_(*basic_condition))
+        or_queries = []
+        for basic_condition in basic_conds['or']:
+            or_queries.append(*basic_condition)
 
     if tag_conds:
         for tag_condition in tag_conds:
@@ -264,9 +268,15 @@ def _apply_user_filters(query, basic_conds, tag_conds, prop_conds):
                 and_(*tag_condition))
 
     if prop_conds:
-        for prop_condition in prop_conds:
+        for prop_condition in prop_conds['and']:
             query = query.join(models.ArtifactProperty, aliased=True).filter(
                 and_(*prop_condition))
+        for prop_condition in prop_conds['or']:
+            or_queries.append(and_(*prop_condition))
+
+    if len(or_queries) != 0:
+        query = query.join(models.ArtifactProperty, aliased=True).filter(
+            or_(*or_queries))
 
     return query
 
@@ -423,10 +433,16 @@ op_mappings = {
 
 
 def _do_query_filters(filters):
-    basic_conds = []
+    basic_conds = {
+        "and": [],
+        "or": []
+    }
     tag_conds = []
-    prop_conds = []
-    for field_name, key_name, op, field_type, value in filters:
+    prop_conds = {
+        "and": [],
+        "or": []
+    }
+    for field_name, key_name, op, field_type, value, query_combiner in filters:
         if field_name == 'tags':
             tags = utils.split_filter_value_for_quotes(value)
             for tag in tags:
@@ -438,21 +454,21 @@ def _do_query_filters(filters):
             if op == 'in':
                 if field_name == 'version':
                     value = [semver_db.parse(val) for val in value]
-                    basic_conds.append(
+                    basic_conds[query_combiner].append(
                         [or_(*[
                             models.Artifact.version == ver for ver in value])])
                 else:
-                    basic_conds.append(
+                    basic_conds[query_combiner].append(
                         [getattr(models.Artifact, field_name).in_(value)])
             elif op == 'like':
-                basic_conds.append(
+                basic_conds[query_combiner].append(
                     [getattr(models.Artifact, field_name).like(value)])
             else:
                 fn = op_mappings[op]
                 if field_name == 'version':
                     value = semver_db.parse(value)
-                basic_conds.append([fn(getattr(models.Artifact, field_name),
-                                       value)])
+                basic_conds[query_combiner].append(
+                    [fn(getattr(models.Artifact, field_name), value)])
         else:
             conds = [models.ArtifactProperty.name == field_name]
             if key_name is not None:
@@ -474,7 +490,7 @@ def _do_query_filters(filters):
                     conds.extend([fn(getattr(models.ArtifactProperty,
                                              field_type + '_value'), value)])
 
-            prop_conds.append(conds)
+            prop_conds[query_combiner].append(conds)
 
     return basic_conds, tag_conds, prop_conds
 
@@ -772,11 +788,12 @@ def delete_blob_data(context, uri, session):
             models.ArtifactBlobData).filter_by(id=blob_data_id).delete()
 
 
-def get_artifact_count(context, session, filters=None, latest=False):
+def get_artifact_count(context, session, filters=None, latest=False,
+                       list_all_artifacts=False):
 
     filters = filters or {}
 
-    query = _create_artifact_count_query(context, session)
+    query = _create_artifact_count_query(context, session, list_all_artifacts)
 
     basic_conds, tag_conds, prop_conds = _do_query_filters(filters)
 
@@ -789,8 +806,9 @@ def get_artifact_count(context, session, filters=None, latest=False):
     return query.all()[0].total_count
 
 
-def _create_artifact_count_query(context, session):
+def _create_artifact_count_query(context, session, list_all_artifacts):
 
-    query = session.query(func.count(models.Artifact.id).label("total_count"))
+    query = session.query(func.count(distinct(models.Artifact.id))
+                          .label("total_count"))
 
-    return _apply_query_base_filters(query, context)
+    return _apply_query_base_filters(query, context, list_all_artifacts)

glare/objects/base.py

@@ -71,6 +71,8 @@ class BaseArtifact(base.VersionedObject):
 
     STATUS = ('drafted', 'active', 'deactivated', 'deleted')
 
+    DEFAULT_QUERY_COMBINER = "and"
+
     Field = wrappers.Field.init
     DictField = wrappers.DictField.init
     ListField = wrappers.ListField.init
@@ -356,7 +358,8 @@ Possible values:
                     msg = _("Tags are filtered without operator")
                     raise exception.BadRequest(msg)
                 new_filters.append(
-                    (filter_name, None, None, None, filter_value))
+                    (filter_name, None, None, None, filter_value,
+                     cls.DEFAULT_QUERY_COMBINER))
                 continue
 
             key_name = None
@@ -377,19 +380,25 @@ Possible values:
                 field_type = field_type.element_type
 
             try:
-                op, val = utils.split_filter_op(filter_value)
+                query_combiner, op, val = utils.split_filter_op(filter_value)
+
                 if isinstance(field_type, glare_fields.Dict):
                     if op not in ['eq', 'in']:
                         msg = (_("Unsupported filter type '%s'. The following "
                                  "filters are supported: eq, in") % op)
                         raise exception.BadRequest(message=msg)
+                    if query_combiner not in ["and", "or"]:
+                        msg = (_("Unsupported Query combiner type '%s'. Only "
+                                 "following combiner are allowed: and, or")
+                               % query_combiner)
+                        raise exception.BadRequest(message=msg)
                     if op == 'in':
                         new_filters.append((
                             filter_name, utils.split_filter_value_for_quotes(
-                                val), op, None, None))
+                                val), op, None, None, query_combiner))
                     else:
                         new_filters.append((
-                            filter_name, val, op, None, None))
+                            filter_name, val, op, None, None, query_combiner))
                 else:
                     cls._validate_filter_ops(filter_name, op)
                     if op == 'in':
@@ -400,7 +409,8 @@ Possible values:
                         value = field_type.coerce(cls(), filter_name, val)
                     new_filters.append(
                         (filter_name, key_name, op,
-                         cls._get_field_type(field_type), value))
+                         cls._get_field_type(field_type),
+                         value, query_combiner))
             except ValueError:
                 msg = _("Invalid filter value: %s") % str(val)
                 raise exception.BadRequest(msg)
@@ -440,10 +450,12 @@ Possible values:
                 sort.append(default_sort)
 
         default_filter_parameters = [
-            ('status', None, 'neq', None, 'deleted')]
+            ('status', None, 'neq', None, 'deleted',
+             cls.DEFAULT_QUERY_COMBINER)]
         if cls.get_type_name() != 'all':
             default_filter_parameters.append(
-                ('type_name', None, 'eq', None, cls.get_type_name()))
+                ('type_name', None, 'eq', None, cls.get_type_name(),
+                 cls.DEFAULT_QUERY_COMBINER))
         # Parse filter parameters and update them with defaults
         filters = [] if filters is None else cls._parse_filter_values(filters)
         for default_filter in default_filter_parameters:

glare/scrubber.py

@@ -148,7 +148,7 @@ class Scrubber(object):
                 session=db_api.get_session(),
                 limit=CONF.scrubber.scrub_pool_size,
                 sort=[],
-                filters=[('status', None, 'eq', None, 'deleted')])
+                filters=[('status', None, 'eq', None, 'deleted', 'and')])
             if not artifacts:
                 break
             self.pool.imap(self._scrub_artifact, artifacts)

glare/tests/functional/test_sample_artifact.py

@@ -273,6 +273,10 @@ class TestList(base.TestArtifact):
         result = sort_results(self.get(url=url)['artifacts'])
         self.assertEqual(art_list[5:], result)
 
+        url = '/sample_artifact?bool1=False'
+        result = sort_results(self.get(url=url)['artifacts'])
+        self.assertEqual(art_list[5:], result)
+
         # Like filter test cases for name, status
         url = '/sample_artifact?name=like:name%'
         artifacts = self.get(url=url)['artifacts']
@@ -665,6 +669,52 @@ class TestList(base.TestArtifact):
         self.assertEqual(res['total_count'], 0)
         self.assertEqual(res['display_type_name'], "Sample Artifact")
 
+    def test_list_artifact_with_filter_query_combiner(self):
+        # Create artifact
+        art_list = [self.create_artifact({'name': 'name%s' % i,
+                                          'version': '2.0',
+                                          'tags': ['tag%s' % i],
+                                          'int1': 1024,
+                                          'float1': 123.456,
+                                          'str1': 'bugaga',
+                                          'bool1': True})
+                    for i in range(5)]
+
+        public_art = self.create_artifact({'name': 'name5',
+                                           'version': '2.0',
+                                           'tags': ['tag4', 'tag5'],
+                                           'int1': 2048,
+                                           'float1': 987.654,
+                                           'str1': 'lalala',
+                                           'bool1': False,
+                                           'string_required': '123'})
+        url = '/sample_artifact/%s' % public_art['id']
+        data = [{
+            "op": "replace",
+            "path": "/status",
+            "value": "active"
+        }]
+
+        self.patch(url=url, data=data, status=200)
+        public_art = self.admin_action(public_art['id'], self.make_public)
+
+        art_list.append(public_art)
+
+        url = '/sample_artifact?float1=and:lte:123.456&str1=or:eq:lalal&' \
+              'str1=or:eq:bugaga'
+        result = sort_results(self.get(url=url)['artifacts'])
+        self.assertEqual(art_list[:5], result)
+
+        url = '/sample_artifact?str1=or:blah:t'
+        self.get(url=url, status=400)
+
+        url = '/sample_artifact?str1=or:blabla:t:tt'
+        self.get(url=url, status=400)
+
+        url = '/sample_artifact?str1=or:eq:blabla:t:tt'
+        result = self.get(url=url)['artifacts']
+        self.assertEqual([], result)
+
 
 class TestBlobs(base.TestArtifact):
     def test_blob_dicts(self):

glare/tests/unit/api/test_list.py

@@ -170,6 +170,13 @@ class TestArtifactList(base.BaseTestArtifactAPI):
         self.assertRaises(exc.BadRequest, self.controller.list,
                           self.req, 'sample_artifact', filters)
 
+        # Filter by or operation
+        filters = [('float1', 'or:5.0'), ('bool1', 'or:yes')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(6, len(res['artifacts']))
+        for i in (0, 1, 2, 3, 4, 6):
+            self.assertIn(arts[i], res['artifacts'])
+
     def test_list_marker_and_limit(self):
         # Create artifacts
         art_list = [
@@ -463,9 +470,9 @@ class TestArtifactList(base.BaseTestArtifactAPI):
             self.assertIn(arts[i], res['artifacts'])
 
         # Filter with invalid operator leads to BadRequest
-        filters = [('list_of_str', 'invalid:aa')]
-        self.assertRaises(exc.BadRequest, self.controller.list,
-                          self.req, 'sample_artifact', filters)
+        # filters = [('list_of_str', 'invalid:aa')]
+        # self.assertRaises(exc.BadRequest, self.controller.list,
+        #                   self.req, 'sample_artifact', filters)
 
         # Return artifacts that contain key 1 in 'list_of_int'
         filters = [('list_of_int', 'eq:1')]
@@ -615,3 +622,26 @@ class TestArtifactList(base.BaseTestArtifactAPI):
         filters = [('str1', 'like:%haha%')]
         res = self.controller.list(self.req, 'sample_artifact', filters)
         self.assertEqual(0, len(res['artifacts']))
+
+    def test_list_query_combiner(self):
+        values = [{'name': 'combiner0', 'str1': 'banana'},
+                  {'name': 'combiner1', 'str1': 'nan'},
+                  {'name': 'combiner2', 'str1': 'anab'},
+                  {'name': 'combiner3', 'str1': 'blabla'}]
+
+        [self.controller.create(self.req, 'sample_artifact', val)
+         for val in values]
+
+        filters = [('str1', 'or:nan'), ('name', 'or:combiner3')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(2, len(res['artifacts']))
+        self.assertEqual(2, res['total_count'])
+
+        filters = [('str1', 'or:like:%nan%'), ('str1', 'or:blabla')]
+        res = self.controller.list(self.req, 'sample_artifact', filters)
+        self.assertEqual(3, len(res['artifacts']))
+        self.assertEqual(3, res['total_count'])
+
+        filters = [('name', 'or:tt:ttt'), ('str1', "or:blabla")]
+        self.assertRaises(exc.BadRequest, self.controller.list,
+                          self.req, 'sample_artifact', filters)