Filter cgsnapshots data on the DB side

It filters cgsnapshots data on the DB side to improve performance.

Change-Id: I399ddec9d21345bf0496a293059963363c7c8cbe
Partial-Implements: blueprint data-filtering-on-the-db-side

diff --git a/cinder/consistencygroup/api.py b/cinder/consistencygroup/api.py
index 6d7c5e036c0b0ae2cb2a045e8190397d808a08d1..9b56001575526b4f77df7d6c15bd63b30e191e62 100644 (file)
--- a/cinder/consistencygroup/api.py
+++ b/cinder/consistencygroup/api.py
@@ -686,21 +686,9 @@ class API(base.Base):
         if (context.is_admin and 'all_tenants' in search_opts):
             # Need to remove all_tenants to pass the filtering below.
             del search_opts['all_tenants']
-            cgsnapshots = self.db.cgsnapshot_get_all(context)
+            cgsnapshots = self.db.cgsnapshot_get_all(context, search_opts)
         else:
             cgsnapshots = self.db.cgsnapshot_get_all_by_project(
-                context.elevated(), context.project_id)
-
-        if search_opts:
-            LOG.debug("Searching by: %s", search_opts)
-
-            results = []
-            not_found = object()
-            for cgsnapshot in cgsnapshots:
-                for opt, value in search_opts.items():
-                    if cgsnapshot.get(opt, not_found) != value:
-                        break
-                else:
-                    results.append(cgsnapshot)
-            cgsnapshots = results
+                context.elevated(), context.project_id, search_opts)
+
         return cgsnapshots

diff --git a/cinder/db/api.py b/cinder/db/api.py
index 0933888c32969d63b3370b4aed1fc761c98a9a38..3c5779d81433dc5eb529fff0f7ccdcf4a597f9c1 100644 (file)
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -936,9 +936,9 @@ def cgsnapshot_get(context, cgsnapshot_id):
     return IMPL.cgsnapshot_get(context, cgsnapshot_id)
 
 
-def cgsnapshot_get_all(context):
+def cgsnapshot_get_all(context, filters=None):
     """Get all cgsnapshots."""
-    return IMPL.cgsnapshot_get_all(context)
+    return IMPL.cgsnapshot_get_all(context, filters)
 
 
 def cgsnapshot_create(context, values):
@@ -946,14 +946,14 @@ def cgsnapshot_create(context, values):
     return IMPL.cgsnapshot_create(context, values)
 
 
-def cgsnapshot_get_all_by_group(context, group_id):
+def cgsnapshot_get_all_by_group(context, group_id, filters=None):
     """Get all cgsnapshots belonging to a consistency group."""
-    return IMPL.cgsnapshot_get_all_by_group(context, group_id)
+    return IMPL.cgsnapshot_get_all_by_group(context, group_id, filters)
 
 
-def cgsnapshot_get_all_by_project(context, project_id):
+def cgsnapshot_get_all_by_project(context, project_id, filters=None):
     """Get all cgsnapshots belonging to a project."""
-    return IMPL.cgsnapshot_get_all_by_project(context, project_id)
+    return IMPL.cgsnapshot_get_all_by_project(context, project_id, filters)
 
 
 def cgsnapshot_update(context, cgsnapshot_id, values):

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index dffb2f083bd3208a54d9dfaa5ea18d706867e368..02208a54447c6ef3fc518764d3e131e1f1c7282f 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -3601,23 +3601,52 @@ def cgsnapshot_get(context, cgsnapshot_id):
     return _cgsnapshot_get(context, cgsnapshot_id)
 
 
+def is_valid_model_filters(model, filters):
+    """Return True if filter values exist on the model
+
+    :param model: a Cinder model
+    :param filters: dictionary of filters
+    """
+    for key in filters.keys():
+        try:
+            getattr(model, key)
+        except AttributeError:
+            LOG.debug("'%s' filter key is not valid.", key)
+            return False
+    return True
+
+
+def _cgsnapshot_get_all(context, project_id=None, group_id=None, filters=None):
+    query = model_query(context, models.Cgsnapshot)
+
+    if filters:
+        if not is_valid_model_filters(models.Cgsnapshot, filters):
+            return []
+        query = query.filter_by(**filters)
+
+    if project_id:
+        query.filter_by(project_id=project_id)
+
+    if group_id:
+        query.filter_by(consistencygroup_id=group_id)
+
+    return query.all()
+
+
 @require_admin_context
-def cgsnapshot_get_all(context):
-    return model_query(context, models.Cgsnapshot).all()
+def cgsnapshot_get_all(context, filters=None):
+    return _cgsnapshot_get_all(context, filters=filters)
 
 
 @require_admin_context
-def cgsnapshot_get_all_by_group(context, group_id):
-    return model_query(context, models.Cgsnapshot).\
-        filter_by(consistencygroup_id=group_id).all()
+def cgsnapshot_get_all_by_group(context, group_id, filters=None):
+    return _cgsnapshot_get_all(context, group_id=group_id, filters=filters)
 
 
 @require_context
-def cgsnapshot_get_all_by_project(context, project_id):
+def cgsnapshot_get_all_by_project(context, project_id, filters=None):
     authorize_project_context(context, project_id)
-
-    return model_query(context, models.Cgsnapshot).\
-        filter_by(project_id=project_id).all()
+    return _cgsnapshot_get_all(context, project_id=project_id, filters=filters)
 
 
 @require_context

diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py
index b8abe0cc139ee889d11196e42747e98665b3979f..b80a850cce3b505d8e77e2ce46e776d53ead7f71 100644 (file)
--- a/cinder/tests/unit/test_db_api.py
+++ b/cinder/tests/unit/test_db_api.py
@@ -28,7 +28,6 @@ from cinder import exception
 from cinder import quota
 from cinder import test
 
-
 CONF = cfg.CONF
 
 THREE = 3
@@ -1187,6 +1186,57 @@ class DBAPISnapshotTestCase(BaseTest):
         self.assertEqual(should_be, db.snapshot_metadata_get(self.ctxt, 1))
 
 
+class DBAPICgsnapshotTestCase(BaseTest):
+    """Tests for cinder.db.api.cgsnapshot_*."""
+
+    def test_cgsnapshot_get_all_by_filter(self):
+        cgsnapshot1 = db.cgsnapshot_create(self.ctxt, {'id': 1,
+                                           'consistencygroup_id': 'g1'})
+        cgsnapshot2 = db.cgsnapshot_create(self.ctxt, {'id': 2,
+                                           'consistencygroup_id': 'g1'})
+        cgsnapshot3 = db.cgsnapshot_create(self.ctxt, {'id': 3,
+                                           'consistencygroup_id': 'g2'})
+        tests = [
+            ({'consistencygroup_id': 'g1'}, [cgsnapshot1, cgsnapshot2]),
+            ({'id': 3}, [cgsnapshot3]),
+            ({'fake_key': 'fake'}, [])
+        ]
+
+        # no filter
+        filters = None
+        cgsnapshots = db.cgsnapshot_get_all(self.ctxt, filters=filters)
+        self.assertEqual(3, len(cgsnapshots))
+
+        for filters, expected in tests:
+            self._assertEqualListsOfObjects(expected,
+                                            db.cgsnapshot_get_all(
+                                                self.ctxt,
+                                                filters))
+
+    def test_cgsnapshot_get_all_by_project(self):
+        cgsnapshot1 = db.cgsnapshot_create(self.ctxt,
+                                           {'id': 1,
+                                            'consistencygroup_id': 'g1',
+                                            'project_id': 1})
+        cgsnapshot2 = db.cgsnapshot_create(self.ctxt,
+                                           {'id': 2,
+                                            'consistencygroup_id': 'g1',
+                                            'project_id': 1})
+        project_id = 1
+        tests = [
+            ({'id': 1}, [cgsnapshot1]),
+            ({'consistencygroup_id': 'g1'}, [cgsnapshot1, cgsnapshot2]),
+            ({'fake_key': 'fake'}, [])
+        ]
+
+        for filters, expected in tests:
+            self._assertEqualListsOfObjects(expected,
+                                            db.cgsnapshot_get_all_by_project(
+                                                self.ctxt,
+                                                project_id,
+                                                filters))
+
+
 class DBAPIVolumeTypeTestCase(BaseTest):
 
     """Tests for the db.api.volume_type_* methods."""