Merge "Fix like filter related issues"

cinder/api/common.py

@@ -69,6 +69,9 @@ _FILTERS_COLLECTION = None
 FILTERING_VERSION = '3.31'
 LIKE_FILTER_VERSION = '3.34'
 
+ATTRIBUTE_CONVERTERS = {'name~': 'display_name~',
+                        'description~': 'display_description~'}
+
 
 METADATA_TYPES = enum.Enum('METADATA_TYPES', 'user image')
 
@@ -447,6 +450,14 @@ def get_enabled_resource_filters(resource=None):
         return {}
 
 
+def convert_filter_attributes(filters, resource):
+    for key in filters.copy().keys():
+        if resource in ['volume', 'backup',
+                        'snapshot'] and key in ATTRIBUTE_CONVERTERS.keys():
+            filters[ATTRIBUTE_CONVERTERS[key]] = filters[key]
+            filters.pop(key)
+
+
 def reject_invalid_filters(context, filters, resource,
                            enable_like_filter=False):
     if context.is_admin:
@@ -464,9 +475,9 @@ def reject_invalid_filters(context, filters, resource,
             if key not in configured_filters:
                 invalid_filters.append(key)
         else:
-            # If 'key~' is configured, both 'key' and 'key~' is valid.
-            if (key not in configured_filters or
-                    "%s~" % key not in configured_filters):
+            # If 'key~' is configured, both 'key' and 'key~' are valid.
+            if not (key in configured_filters or
+                    "%s~" % key in configured_filters):
                 invalid_filters.append(key)
     if invalid_filters:
         raise webob.exc.HTTPBadRequest(
@@ -486,6 +497,8 @@ def process_general_filtering(resource):
                     support_like = True
                 reject_invalid_filters(context, filters,
                                        resource, support_like)
+                convert_filter_attributes(filters, resource)
+
             else:
                 process_non_general_filtering(*args, **kwargs)
         return _decorator

cinder/db/sqlalchemy/api.py

@@ -1736,14 +1736,15 @@ def _process_model_like_filter(model, query, filters):
     if query is None:
         return query
 
-    for key in filters:
+    for key in sorted(filters):
         column_attr = getattr(model, key)
         if 'property' == type(column_attr).__name__:
             continue
         value = filters[key]
-        if not isinstance(value, six.string_types):
+        if not (isinstance(value, six.string_types) or isinstance(value, int)):
             continue
-        query = query.filter(column_attr.op('LIKE')(u'%' + value + u'%'))
+        query = query.filter(
+            column_attr.op('LIKE')(u'%%%s%%' % value))
     return query
 
 

cinder/tests/unit/api/test_common.py

@@ -412,6 +412,65 @@ class GeneralFiltersTest(test.TestCase):
                 common.reject_invalid_filters, fake_context,
                 filters, 'fake_resource')
 
+    @ddt.data({'filters': {'name': 'value1'},
+               'is_admin': False,
+               'result': {'fake_resource': ['name']},
+               'expected': {'name': 'value1'}},
+              {'filters': {'name~': 'value1'},
+               'is_admin': False,
+               'result': {'fake_resource': ['name']},
+               'expected': None},
+              {'filters': {'name': 'value1'},
+               'is_admin': False,
+               'result': {'fake_resource': ['name~']},
+               'expected': {'name': 'value1'}},
+              {'filters': {'name~': 'value1'},
+               'is_admin': False,
+               'result': {'fake_resource': ['name~']},
+               'expected': {'name~': 'value1'}}
+              )
+    @ddt.unpack
+    @mock.patch('cinder.api.common.get_enabled_resource_filters')
+    def test_reject_invalid_filters_like_operator_enabled(
+            self, mock_get, filters, is_admin, result, expected):
+        class FakeContext(object):
+            def __init__(self, admin):
+                self.is_admin = admin
+
+        fake_context = FakeContext(is_admin)
+        mock_get.return_value = result
+        if expected:
+            common.reject_invalid_filters(fake_context,
+                                          filters, 'fake_resource', True)
+            self.assertEqual(expected, filters)
+        else:
+            self.assertRaises(
+                webob.exc.HTTPBadRequest,
+                common.reject_invalid_filters, fake_context,
+                filters, 'fake_resource')
+
+    @ddt.data({'resource': 'group',
+               'filters': {'name~': 'value'},
+               'expected': {'name~': 'value'}},
+              {'resource': 'snapshot',
+               'filters': {'status~': 'value'},
+               'expected': {'status~': 'value'}},
+              {'resource': 'volume',
+               'filters': {'name~': 'value',
+                           'description~': 'value'},
+               'expected': {'display_name~': 'value',
+                            'display_description~': 'value'}},
+              {'resource': 'backup',
+               'filters': {'name~': 'value',
+                           'description~': 'value'},
+               'expected': {'display_name~': 'value',
+                            'display_description~': 'value'}},
+              )
+    @ddt.unpack
+    def test_convert_filter_attributes(self, resource, filters, expected):
+        common.convert_filter_attributes(filters, resource)
+        self.assertEqual(expected, filters)
+
 
 @ddt.ddt
 class LinkPrefixTest(test.TestCase):

cinder/tests/unit/test_db_api.py

@@ -96,6 +96,7 @@ class DBCommonFilterTestCase(BaseTest):
     def test__process_model_like_filter(self, mock_filter):
         filters = {'display_name': 'fake_name',
                    'display_description': 'fake_description',
+                   'host': 123,
                    'status': []}
         session = sqlalchemy_api.get_session()
         query = session.query(models.Volume)
@@ -106,7 +107,8 @@ class DBCommonFilterTestCase(BaseTest):
             mock_op.return_value = fake_operator
             sqlalchemy_api._process_model_like_filter(models.Volume,
                                                       query, filters)
-            calls = [call('%fake_name%'), call('%fake_description%')]
+            calls = [call('%fake_description%'),
+                     call('%fake_name%'), call('%123%')]
             mock_filter.assert_has_calls(calls)
 
     @ddt.data({'handler': [db.volume_create, db.volume_get_all],