From cb5aaf0bcb894a141a9bfb50b9aff4fb209fc850 Mon Sep 17 00:00:00 2001 From: TommyLike Date: Sun, 5 Mar 2017 14:51:00 +0800 Subject: [PATCH] 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 | 3 +- .../openstack/rest_api_version_history.rst | 4 + cinder/api/v3/group_snapshots.py | 22 ++- cinder/api/v3/views/group_snapshots.py | 4 + cinder/db/api.py | 24 ++- cinder/db/sqlalchemy/api.py | 83 +++++--- cinder/group/api.py | 19 +- cinder/objects/group_snapshot.py | 34 ++-- .../tests/unit/api/v3/test_group_snapshots.py | 183 +++++++++++++++++- cinder/tests/unit/group/test_groups_api.py | 2 +- ...r-to-group-snapshots-74sd8g138a289dh4.yaml | 3 + 11 files changed, 318 insertions(+), 63 deletions(-) create mode 100644 releasenotes/notes/add-filter-to-group-snapshots-74sd8g138a289dh4.yaml diff --git a/cinder/api/openstack/api_version_request.py b/cinder/api/openstack/api_version_request.py index ded5af870..168058061 100644 --- a/cinder/api/openstack/api_version_request.py +++ b/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" diff --git a/cinder/api/openstack/rest_api_version_history.rst b/cinder/api/openstack/rest_api_version_history.rst index c85555963..579413342 100644 --- a/cinder/api/openstack/rest_api_version_history.rst +++ b/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. diff --git a/cinder/api/v3/group_snapshots.py b/cinder/api/v3/group_snapshots.py index 56487fdae..adb36a815 100644 --- a/cinder/api/v3/group_snapshots.py +++ b/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) diff --git a/cinder/api/v3/views/group_snapshots.py b/cinder/api/v3/views/group_snapshots.py index ac30724ff..ef7cdc457 100644 --- a/cinder/api/v3/views/group_snapshots.py +++ b/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 diff --git a/cinder/db/api.py b/cinder/db/api.py index 944a6049e..9a28c681d 100644 --- a/cinder/db/api.py +++ b/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): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 1f4b67a0e..6feaf1e25 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/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, @@ -5524,7 +5538,7 @@ def _group_get_all(context, filters=None, marker=None, limit=None, limit, sort_keys, sort_dirs, filters, offset, models.Group) - return query.all()if query else [] + return query.all() if query else [] @require_admin_context @@ -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, - filters=None): - query = model_query(context, models.GroupSnapshot) +def _group_snapshot_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.GroupSnapshot, + filters): + return [] - if filters: - if not is_valid_model_filters(models.GroupSnapshot, filters): - return [] - query = query.filter_by(**filters) + session = get_session() + with session.begin(): + # Generate the paginate query + query = _generate_paginate_query(context, session, marker, + limit, sort_keys, sort_dirs, filters, + offset, models.GroupSnapshot) - if project_id: - query = query.filter_by(project_id=project_id) + 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) - - return query.all() - - -@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) + filters['group_id'] = group_id + return _group_snapshot_get_all(context, filters, marker, limit, offset, + sort_keys, sort_dirs) @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, - filters=filters) + if filters is None: + 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), diff --git a/cinder/group/api.py b/cinder/group/api.py index 22e3b0d3f..6c87b6f96 100644 --- a/cinder/group/api.py +++ b/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'] - group_snapshots = objects.GroupSnapshotList.get_all(context, - search_opts) + del filters['all_tenants'] + group_snapshots = objects.GroupSnapshotList.get_all( + 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): diff --git a/cinder/objects/group_snapshot.py b/cinder/objects/group_snapshot.py index 443e00ea9..70f4420bc 100644 --- a/cinder/objects/group_snapshot.py +++ b/cinder/objects/group_snapshot.py @@ -138,23 +138,35 @@ class GroupSnapshotList(base.ObjectListBase, base.CinderObject): } @classmethod - def get_all(cls, context, filters=None): - group_snapshots = db.group_snapshot_get_all(context, filters) + def get_all(cls, context, filters=None, marker=None, limit=None, + 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): - group_snapshots = db.group_snapshot_get_all_by_project(context, - project_id, - filters) + def get_all_by_project(cls, context, project_id, filters=None, marker=None, + limit=None, offset=None, sort_keys=None, + sort_dirs=None): + 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): - group_snapshots = db.group_snapshot_get_all_by_group(context, group_id, - filters) - return base.obj_make_list(context, cls(context), - objects.GroupSnapshot, + def get_all_by_group(cls, context, group_id, filters=None, marker=None, + limit=None, offset=None, sort_keys=None, + sort_dirs=None): + group_snapshots = db.group_snapshot_get_all_by_group( + 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) diff --git a/cinder/tests/unit/api/v3/test_group_snapshots.py b/cinder/tests/unit/api/v3/test_group_snapshots.py index d060be19f..c1695decc 100644 --- a/cinder/tests/unit/api/v3/test_group_snapshots.py +++ b/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']) - self.assertIsNotNone(res_dict['group_snapshots'][index]['name']) + res_dict['group_snapshots'][2 - index]['id']) + 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') diff --git a/cinder/tests/unit/group/test_groups_api.py b/cinder/tests/unit/group/test_groups_api.py index 5c6d77d88..9fe2a4a4a 100644 --- a/cinder/tests/unit/group/test_groups_api.py +++ b/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( diff --git a/releasenotes/notes/add-filter-to-group-snapshots-74sd8g138a289dh4.yaml b/releasenotes/notes/add-filter-to-group-snapshots-74sd8g138a289dh4.yaml new file mode 100644 index 000000000..13c48538b --- /dev/null +++ b/releasenotes/notes/add-filter-to-group-snapshots-74sd8g138a289dh4.yaml @@ -0,0 +1,3 @@ +--- +fixes: + - Add filter, sorter and pagination support in group snapshot listings.