Adding filter options to backup list

This patch adds filter options to backup list.
Like cinder list, now cinder backup-list accept
parameters.
This blueprint is complimented with the client part

https://blueprints.launchpad.net/python-cinderclient
/+spec/add-filter-options-to-backup-list

blueprint: add-filter-options-to-backup-list

DocImpact
Change-Id: I9e095ee44a03744eb3f319323d9130a89196614e

diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py
index 405b874660e2aa9e0e11b939af2c59a8fbeaf801..1b2970051857ede5f7ec532df057b7099854f34d 100644 (file)
--- a/cinder/api/contrib/backups.py
+++ b/cinder/api/contrib/backups.py
@@ -194,10 +194,25 @@ class BackupsController(wsgi.Controller):
         """Returns a detailed list of backups."""
         return self._get_backups(req, is_detail=True)
 
+    @staticmethod
+    def _get_backup_filter_options():
+        """Return volume search options allowed by non-admin."""
+        return ('name', 'status', 'volume_id')
+
     def _get_backups(self, req, is_detail):
         """Returns a list of backups, transformed through view builder."""
         context = req.environ['cinder.context']
-        backups = self.backup_api.get_all(context)
+        filters = req.params.copy()
+
+        utils.remove_invalid_filter_options(context,
+                                            filters,
+                                            self._get_backup_filter_options())
+
+        if 'name' in filters:
+            filters['display_name'] = filters['name']
+            del filters['name']
+
+        backups = self.backup_api.get_all(context, search_opts=filters)
         limited_list = common.limited(backups, req)
 
         if is_detail:

diff --git a/cinder/api/v1/snapshots.py b/cinder/api/v1/snapshots.py
index 1e441114341468e6c7ba3a2f77ff8272c62e6458..54a50b6c53d0d11474b5d7b853a505f9576873ba 100644 (file)
--- a/cinder/api/v1/snapshots.py
+++ b/cinder/api/v1/snapshots.py
@@ -20,7 +20,6 @@ from webob import exc
 
 from cinder.api import common
 from cinder.api.openstack import wsgi
-from cinder.api.v1 import volumes
 from cinder.api import xmlutil
 from cinder import exception
 from cinder.openstack.common import log as logging
@@ -146,8 +145,8 @@ class SnapshotsController(wsgi.Controller):
 
         #filter out invalid option
         allowed_search_options = ('status', 'volume_id', 'display_name')
-        volumes.remove_invalid_options(context, search_opts,
-                                       allowed_search_options)
+        utils.remove_invalid_filter_options(context, search_opts,
+                                            allowed_search_options)
 
         snapshots = self.volume_api.get_all_snapshots(context,
                                                       search_opts=search_opts)

diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py
index 829237046f060e6f36a9cb57c96135608d265b3f..6021473d6b48d41d7fce7540cc680f35b151ca28 100644 (file)
--- a/cinder/api/v1/volumes.py
+++ b/cinder/api/v1/volumes.py
@@ -273,8 +273,9 @@ class VolumeController(wsgi.Controller):
             search_opts['metadata'] = ast.literal_eval(search_opts['metadata'])
 
         context = req.environ['cinder.context']
-        remove_invalid_options(context,
-                               search_opts, self._get_volume_search_options())
+        utils.remove_invalid_filter_options(context,
+                                            search_opts,
+                                            self._get_volume_search_options())
 
         volumes = self.volume_api.get_all(context, marker=None, limit=None,
                                           sort_key='created_at',
@@ -441,19 +442,3 @@ class VolumeController(wsgi.Controller):
 
 def create_resource(ext_mgr):
     return wsgi.Resource(VolumeController(ext_mgr))
-
-
-def remove_invalid_options(context, search_options, allowed_search_options):
-    """Remove search options that are not valid for non-admin API/context."""
-    if context.is_admin:
-        # Allow all options
-        return
-    # Otherwise, strip out all unknown options
-    unknown_options = [opt for opt in search_options
-                       if opt not in allowed_search_options]
-    bad_options = ", ".join(unknown_options)
-    log_msg = _("Removing options '%(bad_options)s'"
-                " from query") % {'bad_options': bad_options}
-    LOG.debug(log_msg)
-    for opt in unknown_options:
-        del search_options[opt]

diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py
index bc74d58fdfe9609b96a14582d650db1ea7025232..efff59ccb18fd8fe707780f397a0698cd9c2499e 100644 (file)
--- a/cinder/api/v2/snapshots.py
+++ b/cinder/api/v2/snapshots.py
@@ -20,7 +20,6 @@ from webob import exc
 
 from cinder.api import common
 from cinder.api.openstack import wsgi
-from cinder.api.v2 import volumes
 from cinder.api import xmlutil
 from cinder import exception
 from cinder.openstack.common import log as logging
@@ -149,8 +148,8 @@ class SnapshotsController(wsgi.Controller):
 
         #filter out invalid option
         allowed_search_options = ('status', 'volume_id', 'name')
-        volumes.remove_invalid_options(context, search_opts,
-                                       allowed_search_options)
+        utils.remove_invalid_filter_options(context, search_opts,
+                                            allowed_search_options)
 
         # NOTE(thingee): v2 API allows name instead of display_name
         if 'name' in search_opts:

diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index 3434a9bb5ff9323b4b55c6f08ac88d25220401c5..d8b0c19b17ff3402a3a690dc36c8e99ecead78a4 100644 (file)
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -214,8 +214,9 @@ class VolumeController(wsgi.Controller):
         params.pop('offset', None)
         filters = params
 
-        remove_invalid_options(context,
-                               filters, self._get_volume_filter_options())
+        utils.remove_invalid_filter_options(context,
+                                            filters,
+                                            self._get_volume_filter_options())
 
         # NOTE(thingee): v2 API allows name instead of display_name
         if 'name' in filters:
@@ -417,18 +418,3 @@ class VolumeController(wsgi.Controller):
 
 def create_resource(ext_mgr):
     return wsgi.Resource(VolumeController(ext_mgr))
-
-
-def remove_invalid_options(context, filters, allowed_search_options):
-    """Remove search options that are not valid for non-admin API/context."""
-    if context.is_admin:
-        # Allow all options
-        return
-    # Otherwise, strip out all unknown options
-    unknown_options = [opt for opt in filters
-                       if opt not in allowed_search_options]
-    bad_options = ", ".join(unknown_options)
-    log_msg = _("Removing options '%s' from query") % bad_options
-    LOG.debug(log_msg)
-    for opt in unknown_options:
-        del filters[opt]

diff --git a/cinder/backup/api.py b/cinder/backup/api.py
index 37851e4bd51248662564b49be2210d3965b6676a..e0c9a31debd7039ec85afdb1b29eec875be21a4f 100644 (file)
--- a/cinder/backup/api.py
+++ b/cinder/backup/api.py
@@ -71,16 +71,16 @@ class API(base.Base):
                                          backup['host'],
                                          backup['id'])
 
-    # TODO(moorehef): Add support for search_opts, discarded atm
     def get_all(self, context, search_opts=None):
         if search_opts is None:
             search_opts = {}
         check_policy(context, 'get_all')
         if context.is_admin:
-            backups = self.db.backup_get_all(context)
+            backups = self.db.backup_get_all(context, filters=search_opts)
         else:
             backups = self.db.backup_get_all_by_project(context,
-                                                        context.project_id)
+                                                        context.project_id,
+                                                        filters=search_opts)
 
         return backups
 

diff --git a/cinder/db/api.py b/cinder/db/api.py
index b81b04a38114ebc27952e59d096cb717d35bb7ce..ffb92e18064f6f914492c0888acff365d7c5abad 100644 (file)
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -722,9 +722,9 @@ def backup_get(context, backup_id):
     return IMPL.backup_get(context, backup_id)
 
 
-def backup_get_all(context):
+def backup_get_all(context, filters=None):
     """Get all backups."""
-    return IMPL.backup_get_all(context)
+    return IMPL.backup_get_all(context, filters=filters)
 
 
 def backup_get_all_by_host(context, host):
@@ -737,9 +737,10 @@ def backup_create(context, values):
     return IMPL.backup_create(context, values)
 
 
-def backup_get_all_by_project(context, project_id):
+def backup_get_all_by_project(context, project_id, filters=None):
     """Get all backups belonging to a project."""
-    return IMPL.backup_get_all_by_project(context, project_id)
+    return IMPL.backup_get_all_by_project(context, project_id,
+                                          filters=filters)
 
 
 def backup_update(context, backup_id, values):

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 219740b1c8f5e16684a3b27e4b6e68baab57f2fb..f9bac15c6755f13fbd13906c02d2e326a548e4c2 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -2567,9 +2567,20 @@ def backup_get(context, backup_id):
     return result
 
 
+def _backup_get_all(context, filters=None):
+    session = get_session()
+    with session.begin():
+        # Generate the query
+        query = model_query(context, models.Backup)
+        if filters:
+            query = query.filter_by(**filters)
+
+        return query.all()
+
+
 @require_admin_context
-def backup_get_all(context):
-    return model_query(context, models.Backup).all()
+def backup_get_all(context, filters=None):
+    return _backup_get_all(context, filters)
 
 
 @require_admin_context
@@ -2578,11 +2589,17 @@ def backup_get_all_by_host(context, host):
 
 
 @require_context
-def backup_get_all_by_project(context, project_id):
+def backup_get_all_by_project(context, project_id, filters=None):
+
     authorize_project_context(context, project_id)
+    if not filters:
+        filters = {}
+    else:
+        filters = filters.copy()
+
+    filters['project_id'] = project_id
 
-    return model_query(context, models.Backup).\
-        filter_by(project_id=project_id).all()
+    return _backup_get_all(context, filters)
 
 
 @require_context

diff --git a/cinder/tests/api/contrib/test_backups.py b/cinder/tests/api/contrib/test_backups.py
index 3cda44a48876d690d3fcf6d261ea15d3423290d1..6fe3a6f36ecb862c3c38152b608fc2aaf5fa1535 100644 (file)
--- a/cinder/tests/api/contrib/test_backups.py
+++ b/cinder/tests/api/contrib/test_backups.py
@@ -251,6 +251,48 @@ class BackupsAPITestCase(test.TestCase):
         db.backup_destroy(context.get_admin_context(), backup_id2)
         db.backup_destroy(context.get_admin_context(), backup_id1)
 
+    def test_list_backups_detail_using_filters(self):
+        backup_id1 = self._create_backup(display_name='test2')
+        backup_id2 = self._create_backup(status='available')
+        backup_id3 = self._create_backup(volume_id=4321)
+
+        req = webob.Request.blank('/v2/fake/backups/detail?name=test2')
+        req.method = 'GET'
+        req.headers['Content-Type'] = 'application/json'
+        req.headers['Accept'] = 'application/json'
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        self.assertEqual(len(res_dict['backups']), 1)
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res_dict['backups'][0]['id'], backup_id1)
+
+        req = webob.Request.blank('/v2/fake/backups/detail?status=available')
+        req.method = 'GET'
+        req.headers['Content-Type'] = 'application/json'
+        req.headers['Accept'] = 'application/json'
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        self.assertEqual(len(res_dict['backups']), 1)
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res_dict['backups'][0]['id'], backup_id2)
+
+        req = webob.Request.blank('/v2/fake/backups/detail?volume_id=4321')
+        req.method = 'GET'
+        req.headers['Content-Type'] = 'application/json'
+        req.headers['Accept'] = 'application/json'
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        self.assertEqual(len(res_dict['backups']), 1)
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(res_dict['backups'][0]['id'], backup_id3)
+
+        db.backup_destroy(context.get_admin_context(), backup_id3)
+        db.backup_destroy(context.get_admin_context(), backup_id2)
+        db.backup_destroy(context.get_admin_context(), backup_id1)
+
     def test_list_backups_detail_xml(self):
         backup_id1 = self._create_backup()
         backup_id2 = self._create_backup()

diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py
index 532ba5a78f793bdb904c4ed171b19dcdd958453b..61a46c0f539d0542215091251ce68b1cecf7b9f4 100644 (file)
--- a/cinder/tests/test_db_api.py
+++ b/cinder/tests/test_db_api.py
@@ -1184,6 +1184,19 @@ class DBAPIBackupTestCase(BaseTest):
         all_backups = db.backup_get_all(self.ctxt)
         self._assertEqualListsOfObjects(self.created, all_backups)
 
+    def tests_backup_get_all_by_filter(self):
+        filters = {'status': self.created[1]['status']}
+        filtered_backups = db.backup_get_all(self.ctxt, filters=filters)
+        self._assertEqualListsOfObjects([self.created[1]], filtered_backups)
+
+        filters = {'display_name': self.created[1]['display_name']}
+        filtered_backups = db.backup_get_all(self.ctxt, filters=filters)
+        self._assertEqualListsOfObjects([self.created[1]], filtered_backups)
+
+        filters = {'volume_id': self.created[1]['volume_id']}
+        filtered_backups = db.backup_get_all(self.ctxt, filters=filters)
+        self._assertEqualListsOfObjects([self.created[1]], filtered_backups)
+
     def test_backup_get_all_by_host(self):
         byhost = db.backup_get_all_by_host(self.ctxt,
                                            self.created[1]['host'])

diff --git a/cinder/utils.py b/cinder/utils.py
index 6bc03835e1a2f9c5866d377797aa2df628d10158..c9603998cf52b0477d3c4b4dff038818b14c59ad 100644 (file)
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -846,3 +846,21 @@ def add_visible_admin_metadata(volume):
         volume['metadata'].update(visible_admin_meta)
     else:
         volume['metadata'] = visible_admin_meta
+
+
+def remove_invalid_filter_options(context, filters,
+                                  allowed_search_options):
+    """Remove search options that are not valid
+    for non-admin API/context.
+    """
+    if context.is_admin:
+        # Allow all options
+        return
+    # Otherwise, strip out all unknown options
+    unknown_options = [opt for opt in filters
+                       if opt not in allowed_search_options]
+    bad_options = ", ".join(unknown_options)
+    log_msg = "Removing options '%s' from query." % bad_options
+    LOG.debug(log_msg)
+    for opt in unknown_options:
+        del filters[opt]