Add filter, sorter and pagination for group snapshot

Add filter, sorter and pagination support in group
snapshot with new microversion v3.29.

APIImpact
Closes-Bug: #1670540

Change-Id: I2ed1b87b022314b157fe432a97783ab50316367b

cinder/api/openstack/api_version_request.py

@@ -79,6 +79,7 @@ REST_API_VERSION_HISTORY = """
              return new data.
     * 3.27 - Add attachment API
     * 3.28 - Add filters support to get_pools
+    * 3.29 - Add filter, sorter and pagination support in group snapshot.
 
 """
 
@@ -87,7 +88,7 @@ REST_API_VERSION_HISTORY = """
 # minimum version of the API supported.
 # Explicitly using /v1 or /v2 enpoints will still work
 _MIN_API_VERSION = "3.0"
-_MAX_API_VERSION = "3.28"
+_MAX_API_VERSION = "3.29"
 _LEGACY_API_VERSION1 = "1.0"
 _LEGACY_API_VERSION2 = "2.0"
 

cinder/api/openstack/rest_api_version_history.rst

@@ -288,3 +288,7 @@ user documentation.
 3.28
 ----
   Add filters support to get_pools
+
+3.29
+----
+  Add filter, sorter and pagination support in group snapshot.

cinder/api/v3/group_snapshots.py

@@ -105,16 +105,23 @@ class GroupSnapshotsController(wsgi.Controller):
 
     def _get_group_snapshots(self, req, is_detail):
         """Returns a list of group_snapshots through view builder."""
-        context = req.environ['cinder.context']
-        group_snapshots = self.group_snapshot_api.get_all_group_snapshots(
-            context)
-        limited_list = common.limited(group_snapshots, req)
 
+        context = req.environ['cinder.context']
+        req_version = req.api_version_request
+        filters = marker = limit = offset = sort_keys = sort_dirs = None
+        if req_version.matches("3.29"):
+            filters = req.params.copy()
+            marker, limit, offset = common.get_pagination_params(filters)
+            sort_keys, sort_dirs = common.get_sort_params(filters)
+        group_snapshots = self.group_snapshot_api.get_all_group_snapshots(
+            context, filters=filters, marker=marker, limit=limit,
+            offset=offset, sort_keys=sort_keys, sort_dirs=sort_dirs)
         if is_detail:
-            group_snapshots = self._view_builder.detail_list(req, limited_list)
+            group_snapshots = self._view_builder.detail_list(req,
+                                                             group_snapshots)
         else:
             group_snapshots = self._view_builder.summary_list(req,
-                                                              limited_list)
+                                                              group_snapshots)
 
         new_group_snapshots = []
         for grp_snap in group_snapshots['group_snapshots']:
@@ -128,7 +135,8 @@ class GroupSnapshotsController(wsgi.Controller):
                 # Skip migrated group snapshot
                 pass
 
-        return {'group_snapshots': new_group_snapshots}
+        group_snapshots['group_snapshots'] = new_group_snapshots
+        return group_snapshots
 
     @wsgi.Controller.api_version(GROUP_SNAPSHOT_API_VERSION)
     @wsgi.response(http_client.ACCEPTED)

cinder/api/v3/views/group_snapshots.py

@@ -63,6 +63,10 @@ class ViewBuilder(common.ViewBuilder):
         """Provide a view for a list of group_snapshots."""
         group_snapshots_list = [func(request, group_snapshot)['group_snapshot']
                                 for group_snapshot in group_snapshots]
+        group_snapshot_links = self._get_collection_links(
+            request, group_snapshots_list, self._collection_name)
         group_snapshots_dict = dict(group_snapshots=group_snapshots_list)
+        if group_snapshot_links:
+            group_snapshots_dict['group_snapshot_links'] = group_snapshot_links
 
         return group_snapshots_dict

cinder/db/api.py

@@ -1485,9 +1485,11 @@ def group_snapshot_get(context, group_snapshot_id):
     return IMPL.group_snapshot_get(context, group_snapshot_id)
 
 
-def group_snapshot_get_all(context, filters=None):
+def group_snapshot_get_all(context, filters=None, marker=None, limit=None,
+                           offset=None, sort_keys=None, sort_dirs=None):
     """Get all group snapshots."""
-    return IMPL.group_snapshot_get_all(context, filters)
+    return IMPL.group_snapshot_get_all(context, filters, marker, limit,
+                                       offset, sort_keys, sort_dirs)
 
 
 def group_snapshot_create(context, values):
@@ -1495,14 +1497,24 @@ def group_snapshot_create(context, values):
     return IMPL.group_snapshot_create(context, values)
 
 
-def group_snapshot_get_all_by_group(context, group_id, filters=None):
+def group_snapshot_get_all_by_group(context, group_id, filters=None,
+                                    marker=None, limit=None,
+                                    offset=None, sort_keys=None,
+                                    sort_dirs=None):
     """Get all group snapshots belonging to a group."""
-    return IMPL.group_snapshot_get_all_by_group(context, group_id, filters)
+    return IMPL.group_snapshot_get_all_by_group(context, group_id,
+                                                filters, marker, limit,
+                                                offset, sort_keys, sort_dirs)
 
 
-def group_snapshot_get_all_by_project(context, project_id, filters=None):
+def group_snapshot_get_all_by_project(context, project_id, filters=None,
+                                      marker=None, limit=None,
+                                      offset=None, sort_keys=None,
+                                      sort_dirs=None):
     """Get all group snapshots belonging to a project."""
-    return IMPL.group_snapshot_get_all_by_project(context, project_id, filters)
+    return IMPL.group_snapshot_get_all_by_project(context, project_id,
+                                                  filters, marker, limit,
+                                                  offset, sort_keys, sort_dirs)
 
 
 def group_snapshot_update(context, group_snapshot_id, values):

cinder/db/sqlalchemy/api.py

@@ -5502,6 +5502,11 @@ def _groups_get_query(context, session=None, project_only=False):
                        project_only=project_only)
 
 
+def _group_snapshot_get_query(context, session=None, project_only=False):
+    return model_query(context, models.GroupSnapshot, session=session,
+                       project_only=project_only)
+
+
 def _process_groups_filters(query, filters):
     if filters:
         # Ensure that filters' keys exist on the model
@@ -5511,6 +5516,15 @@ def _process_groups_filters(query, filters):
     return query
 
 
+def _process_group_snapshot_filters(query, filters):
+    if filters:
+        # Ensure that filters' keys exist on the model
+        if not is_valid_model_filters(models.GroupSnapshot, filters):
+            return
+        query = query.filter_by(**filters)
+    return query
+
+
 def _group_get_all(context, filters=None, marker=None, limit=None,
                    offset=None, sort_keys=None, sort_dirs=None):
     if filters and not is_valid_model_filters(models.Group,
@@ -5917,39 +5931,53 @@ def group_snapshot_get(context, group_snapshot_id):
     return _group_snapshot_get(context, group_snapshot_id)
 
 
-def _group_snapshot_get_all(context, project_id=None, group_id=None,
+def _group_snapshot_get_all(context, filters=None, marker=None, limit=None,
-                            filters=None):
+                            offset=None, sort_keys=None, sort_dirs=None):
-    query = model_query(context, models.GroupSnapshot)
+    if filters and not is_valid_model_filters(models.GroupSnapshot,
-
+                                              filters):
-    if filters:
-        if not is_valid_model_filters(models.GroupSnapshot, filters):
         return []
-        query = query.filter_by(**filters)
 
-    if project_id:
+    session = get_session()
-        query = query.filter_by(project_id=project_id)
+    with session.begin():
+        # Generate the paginate query
+        query = _generate_paginate_query(context, session, marker,
+                                         limit, sort_keys, sort_dirs, filters,
+                                         offset, models.GroupSnapshot)
 
+        return query.all() if query else []
+
+
+@require_admin_context
+def group_snapshot_get_all(context, filters=None, marker=None, limit=None,
+                           offset=None, sort_keys=None, sort_dirs=None):
+
+    return _group_snapshot_get_all(context, filters, marker, limit, offset,
+                                   sort_keys, sort_dirs)
+
+
+@require_admin_context
+def group_snapshot_get_all_by_group(context, group_id, filters=None,
+                                    marker=None, limit=None, offset=None,
+                                    sort_keys=None, sort_dirs=None):
+    if filters is None:
+        filters = {}
     if group_id:
-        query = query.filter_by(group_id=group_id)
+        filters['group_id'] = group_id
-
+    return _group_snapshot_get_all(context, filters, marker, limit, offset,
-    return query.all()
+                                   sort_keys, sort_dirs)
-
-
-@require_admin_context
-def group_snapshot_get_all(context, filters=None):
-    return _group_snapshot_get_all(context, filters=filters)
-
-
-@require_admin_context
-def group_snapshot_get_all_by_group(context, group_id, filters=None):
-    return _group_snapshot_get_all(context, group_id=group_id, filters=filters)
 
 
 @require_context
-def group_snapshot_get_all_by_project(context, project_id, filters=None):
+def group_snapshot_get_all_by_project(context, project_id, filters=None,
+                                      marker=None, limit=None, offset=None,
+                                      sort_keys=None, sort_dirs=None):
     authorize_project_context(context, project_id)
-    return _group_snapshot_get_all(context, project_id=project_id,
+    if filters is None:
-                                   filters=filters)
+        filters = {}
+    if project_id:
+        filters['project_id'] = project_id
+    return _group_snapshot_get_all(context, filters, marker, limit, offset,
+                                   sort_keys, sort_dirs)
 
 
 @handle_db_data_error
@@ -6257,6 +6285,9 @@ PAGINATION_HELPERS = {
     models.Group: (_groups_get_query,
                    _process_groups_filters,
                    _group_get),
+    models.GroupSnapshot: (_group_snapshot_get_query,
+                           _process_group_snapshot_filters,
+                           _group_snapshot_get),
     models.VolumeAttachment: (_attachment_get_query,
                               _process_attachment_filters,
                               _attachment_get),

cinder/group/api.py

@@ -868,18 +868,23 @@ class API(base.Base):
                                                           group_snapshot_id)
         return group_snapshots
 
-    def get_all_group_snapshots(self, context, search_opts=None):
+    def get_all_group_snapshots(self, context, filters=None, marker=None,
+                                limit=None, offset=None, sort_keys=None,
+                                sort_dirs=None):
         check_policy(context, 'get_all_group_snapshots')
-        search_opts = search_opts or {}
+        filters = filters or {}
 
-        if context.is_admin and 'all_tenants' in search_opts:
+        if context.is_admin and 'all_tenants' in filters:
             # Need to remove all_tenants to pass the filtering below.
-            del search_opts['all_tenants']
+            del filters['all_tenants']
-            group_snapshots = objects.GroupSnapshotList.get_all(context,
+            group_snapshots = objects.GroupSnapshotList.get_all(
-                                                                search_opts)
+                context, filters=filters, marker=marker, limit=limit,
+                offset=offset, sort_keys=sort_keys, sort_dirs=sort_dirs)
         else:
             group_snapshots = objects.GroupSnapshotList.get_all_by_project(
-                context.elevated(), context.project_id, search_opts)
+                context.elevated(), context.project_id, filters=filters,
+                marker=marker, limit=limit, offset=offset, sort_keys=sort_keys,
+                sort_dirs=sort_dirs)
         return group_snapshots
 
     def reset_group_snapshot_status(self, context, gsnapshot, status):

cinder/objects/group_snapshot.py

@@ -138,23 +138,35 @@ class GroupSnapshotList(base.ObjectListBase, base.CinderObject):
     }
 
     @classmethod
-    def get_all(cls, context, filters=None):
+    def get_all(cls, context, filters=None, marker=None, limit=None,
-        group_snapshots = db.group_snapshot_get_all(context, filters)
+                offset=None, sort_keys=None, sort_dirs=None):
+        group_snapshots = db.group_snapshot_get_all(context,
+                                                    filters=filters,
+                                                    marker=marker,
+                                                    limit=limit,
+                                                    offset=offset,
+                                                    sort_keys=sort_keys,
+                                                    sort_dirs=sort_dirs)
         return base.obj_make_list(context, cls(context), objects.GroupSnapshot,
                                   group_snapshots)
 
     @classmethod
-    def get_all_by_project(cls, context, project_id, filters=None):
+    def get_all_by_project(cls, context, project_id, filters=None, marker=None,
-        group_snapshots = db.group_snapshot_get_all_by_project(context,
+                           limit=None, offset=None, sort_keys=None,
-                                                               project_id,
+                           sort_dirs=None):
-                                                               filters)
+        group_snapshots = db.group_snapshot_get_all_by_project(
+            context, project_id, filters=filters, marker=marker,
+            limit=limit, offset=offset, sort_keys=sort_keys,
+            sort_dirs=sort_dirs)
         return base.obj_make_list(context, cls(context), objects.GroupSnapshot,
                                   group_snapshots)
 
     @classmethod
-    def get_all_by_group(cls, context, group_id, filters=None):
+    def get_all_by_group(cls, context, group_id, filters=None, marker=None,
-        group_snapshots = db.group_snapshot_get_all_by_group(context, group_id,
+                         limit=None, offset=None, sort_keys=None,
-                                                             filters)
+                         sort_dirs=None):
-        return base.obj_make_list(context, cls(context),
+        group_snapshots = db.group_snapshot_get_all_by_group(
-                                  objects.GroupSnapshot,
+            context, group_id, filters=filters, marker=marker, limit=limit,
+            offset=offset, sort_keys=sort_keys, sort_dirs=sort_dirs)
+        return base.obj_make_list(context, cls(context), objects.GroupSnapshot,
                                   group_snapshots)

cinder/tests/unit/api/v3/test_group_snapshots.py

@@ -35,6 +35,7 @@ from cinder.tests.unit import utils
 import cinder.volume
 
 GROUP_MICRO_VERSION = '3.14'
+SUPPORT_FILTER_VERSION = '3.29'
 
 
 @ddt.ddt
@@ -86,6 +87,179 @@ class GroupSnapshotsAPITestCase(test.TestCase):
 
         group_snapshot.destroy()
 
+    @ddt.data(True, False)
+    def test_list_group_snapshots_with_limit(self, is_detail):
+
+        url = '/v3/%s/group_snapshots?limit=1' % fake.PROJECT_ID
+        if is_detail:
+            url = '/v3/%s/group_snapshots/detail?limit=1' % fake.PROJECT_ID
+        req = fakes.HTTPRequest.blank(url, version=SUPPORT_FILTER_VERSION)
+        if is_detail:
+            res_dict = self.controller.detail(req)
+        else:
+            res_dict = self.controller.index(req)
+
+        self.assertEqual(2, len(res_dict))
+        self.assertEqual(1, len(res_dict['group_snapshots']))
+        self.assertEqual(self.g_snapshots_array[2].id,
+                         res_dict['group_snapshots'][0]['id'])
+        next_link = (
+            'http://localhost/v3/%s/group_snapshots?limit='
+            '1&marker=%s' %
+            (fake.PROJECT_ID, res_dict['group_snapshots'][0]['id']))
+        self.assertEqual(next_link,
+                         res_dict['group_snapshot_links'][0]['href'])
+        if is_detail:
+            self.assertIn('description', res_dict['group_snapshots'][0].keys())
+        else:
+            self.assertNotIn('description',
+                             res_dict['group_snapshots'][0].keys())
+
+    @ddt.data(True, False)
+    def test_list_group_snapshot_with_offset(self, is_detail):
+        url = '/v3/%s/group_snapshots?offset=1' % fake.PROJECT_ID
+        if is_detail:
+            url = '/v3/%s/group_snapshots/detail?offset=1' % fake.PROJECT_ID
+        req = fakes.HTTPRequest.blank(url, version=SUPPORT_FILTER_VERSION)
+        if is_detail:
+            res_dict = self.controller.detail(req)
+        else:
+            res_dict = self.controller.index(req)
+        self.assertEqual(1, len(res_dict))
+        self.assertEqual(2, len(res_dict['group_snapshots']))
+        self.assertEqual(self.g_snapshots_array[1].id,
+                         res_dict['group_snapshots'][0]['id'])
+        self.assertEqual(self.g_snapshots_array[0].id,
+                         res_dict['group_snapshots'][1]['id'])
+        if is_detail:
+            self.assertIn('description', res_dict['group_snapshots'][0].keys())
+        else:
+            self.assertNotIn('description',
+                             res_dict['group_snapshots'][0].keys())
+
+    @ddt.data(True, False)
+    def test_list_group_snapshot_with_offset_out_of_range(self, is_detail):
+        url = ('/v3/%s/group_snapshots?offset=234523423455454' %
+               fake.PROJECT_ID)
+        if is_detail:
+            url = ('/v3/%s/group_snapshots/detail?offset=234523423455454' %
+                   fake.PROJECT_ID)
+        req = fakes.HTTPRequest.blank(url, version=SUPPORT_FILTER_VERSION)
+        if is_detail:
+            self.assertRaises(webob.exc.HTTPBadRequest, self.controller.detail,
+                              req)
+        else:
+            self.assertRaises(webob.exc.HTTPBadRequest, self.controller.index,
+                              req)
+
+    @ddt.data(False, True)
+    def test_list_group_snapshot_with_limit_and_offset(self, is_detail):
+        group_snapshot = utils.create_group_snapshot(
+            self.context,
+            group_id=self.group.id,
+            group_type_id=self.group.group_type_id)
+        url = '/v3/%s/group_snapshots?limit=2&offset=1' % fake.PROJECT_ID
+        if is_detail:
+            url = ('/v3/%s/group_snapshots/detail?limit=2&offset=1' %
+                   fake.PROJECT_ID)
+        req = fakes.HTTPRequest.blank(url, version=SUPPORT_FILTER_VERSION)
+        if is_detail:
+            res_dict = self.controller.detail(req)
+        else:
+            res_dict = self.controller.index(req)
+
+        self.assertEqual(2, len(res_dict))
+        self.assertEqual(2, len(res_dict['group_snapshots']))
+        self.assertEqual(self.g_snapshots_array[2].id,
+                         res_dict['group_snapshots'][0]['id'])
+        self.assertEqual(self.g_snapshots_array[1].id,
+                         res_dict['group_snapshots'][1]['id'])
+        self.assertIsNotNone(res_dict['group_snapshot_links'][0]['href'])
+        if is_detail:
+            self.assertIn('description', res_dict['group_snapshots'][0].keys())
+        else:
+            self.assertNotIn('description',
+                             res_dict['group_snapshots'][0].keys())
+        group_snapshot.destroy()
+
+    @ddt.data(False, True)
+    def test_list_group_snapshot_with_filter(self, is_detail):
+        url = ('/v3/%s/group_snapshots?'
+               'all_tenants=True&id=%s') % (fake.PROJECT_ID,
+                                            self.g_snapshots_array[0].id)
+        if is_detail:
+            url = ('/v3/%s/group_snapshots/detail?'
+                   'all_tenants=True&id=%s') % (fake.PROJECT_ID,
+                                                self.g_snapshots_array[0].id)
+        req = fakes.HTTPRequest.blank(url, version=SUPPORT_FILTER_VERSION,
+                                      use_admin_context=True)
+        if is_detail:
+            res_dict = self.controller.detail(req)
+        else:
+            res_dict = self.controller.index(req)
+
+        self.assertEqual(1, len(res_dict))
+        self.assertEqual(1, len(res_dict['group_snapshots']))
+        self.assertEqual(self.g_snapshots_array[0].id,
+                         res_dict['group_snapshots'][0]['id'])
+        if is_detail:
+            self.assertIn('description', res_dict['group_snapshots'][0].keys())
+        else:
+            self.assertNotIn('description',
+                             res_dict['group_snapshots'][0].keys())
+
+    @ddt.data({'is_detail': True, 'version': GROUP_MICRO_VERSION},
+              {'is_detail': False, 'version': GROUP_MICRO_VERSION},
+              {'is_detail': True, 'version': '3.28'},
+              {'is_detail': False, 'version': '3.28'},)
+    @ddt.unpack
+    def test_list_group_snapshot_with_filter_previous_version(self, is_detail,
+                                                              version):
+        url = ('/v3/%s/group_snapshots?'
+               'all_tenants=True&id=%s') % (fake.PROJECT_ID,
+                                            self.g_snapshots_array[0].id)
+        if is_detail:
+            url = ('/v3/%s/group_snapshots/detail?'
+                   'all_tenants=True&id=%s') % (fake.PROJECT_ID,
+                                                self.g_snapshots_array[0].id)
+        req = fakes.HTTPRequest.blank(url, version=version,
+                                      use_admin_context=True)
+
+        if is_detail:
+            res_dict = self.controller.detail(req)
+        else:
+            res_dict = self.controller.index(req)
+
+        self.assertEqual(1, len(res_dict))
+        self.assertEqual(3, len(res_dict['group_snapshots']))
+
+    @ddt.data(False, True)
+    def test_list_group_snapshot_with_sort(self, is_detail):
+        url = '/v3/%s/group_snapshots?sort=id:asc' % fake.PROJECT_ID
+        if is_detail:
+            url = ('/v3/%s/group_snapshots/detail?sort=id:asc' %
+                   fake.PROJECT_ID)
+        req = fakes.HTTPRequest.blank(url, version=SUPPORT_FILTER_VERSION)
+        expect_result = [snapshot.id for snapshot in self.g_snapshots_array]
+        expect_result.sort()
+        if is_detail:
+            res_dict = self.controller.detail(req)
+        else:
+            res_dict = self.controller.index(req)
+        self.assertEqual(1, len(res_dict))
+        self.assertEqual(3, len(res_dict['group_snapshots']))
+        self.assertEqual(expect_result[0],
+                         res_dict['group_snapshots'][0]['id'])
+        self.assertEqual(expect_result[1],
+                         res_dict['group_snapshots'][1]['id'])
+        self.assertEqual(expect_result[2],
+                         res_dict['group_snapshots'][2]['id'])
+        if is_detail:
+            self.assertIn('description', res_dict['group_snapshots'][0].keys())
+        else:
+            self.assertNotIn('description',
+                             res_dict['group_snapshots'][0].keys())
+
     def test_show_group_snapshot_with_group_snapshot_not_found(self):
         req = fakes.HTTPRequest.blank('/v3/%s/group_snapshots/%s' %
                                       (fake.PROJECT_ID,
@@ -112,14 +286,15 @@ class GroupSnapshotsAPITestCase(test.TestCase):
         self.assertEqual(3, len(res_dict['group_snapshots']))
         for index, snapshot in enumerate(self.g_snapshots_array):
             self.assertEqual(snapshot.id,
-                             res_dict['group_snapshots'][index]['id'])
+                             res_dict['group_snapshots'][2 - index]['id'])
-            self.assertIsNotNone(res_dict['group_snapshots'][index]['name'])
+            self.assertIsNotNone(
+                res_dict['group_snapshots'][2 - index]['name'])
             if is_detail:
                 self.assertIn('description',
-                              res_dict['group_snapshots'][index].keys())
+                              res_dict['group_snapshots'][2 - index].keys())
             else:
                 self.assertNotIn('description',
-                                 res_dict['group_snapshots'][index].keys())
+                                 res_dict['group_snapshots'][2 - index].keys())
 
     @mock.patch(
         'cinder.api.openstack.wsgi.Controller.validate_name_and_description')

cinder/tests/unit/group/test_groups_api.py

@@ -310,7 +310,7 @@ class GroupAPITestCase(test.TestCase):
 
         if is_admin:
             grp_snaps = self.group_api.get_all_group_snapshots(
-                self.ctxt, search_opts={'all_tenants': True})
+                self.ctxt, filters={'all_tenants': True})
             self.assertEqual(fake_group_snaps, grp_snaps)
         else:
             grp_snaps = self.group_api.get_all_group_snapshots(

releasenotes/notes/add-filter-to-group-snapshots-74sd8g138a289dh4.yaml

@@ -0,0 +1,3 @@
+---
+fixes:
+  - Add filter, sorter and pagination support in group snapshot listings.