From 0abdd938707aa458c5f3cd841d332a57f32e9621 Mon Sep 17 00:00:00 2001 From: Ekaterina Chernova Date: Fri, 31 Jul 2015 11:51:07 +0300 Subject: [PATCH] Add category list pagination support Also sorting is added. Default sort is done by category name. Change-Id: I50254cb58f3924a5a1400859ea30d788e572729f Closes-Bug: #1474932 --- murano/api/v1/catalog.py | 91 ++++++++++++++++++------ murano/db/catalog/api.py | 15 +++- murano/tests/unit/api/v1/test_catalog.py | 51 +++++++++++++ murano/tests/unit/db/test_catalog.py | 24 +++++++ 4 files changed, 159 insertions(+), 22 deletions(-) diff --git a/murano/api/v1/catalog.py b/murano/api/v1/catalog.py index 18278e5e..0f4912c1 100644 --- a/murano/api/v1/catalog.py +++ b/murano/api/v1/catalog.py @@ -21,6 +21,7 @@ import jsonschema from oslo_config import cfg from oslo_db import exception as db_exc from oslo_log import log as logging +from oslo_log import versionutils from webob import exc import murano.api.v1 @@ -120,6 +121,23 @@ def _validate_body(body): class Controller(object): """WSGI controller for application catalog resource in Murano v1 API.""" + def _validate_limit(self, value): + if value is None: + return + try: + value = int(value) + except ValueError: + msg = _("limit param must be an integer") + LOG.error(msg) + raise exc.HTTPBadRequest(explanation=msg) + + if value <= 0: + msg = _("limit param must be positive") + LOG.error(msg) + raise exc.HTTPBadRequest(explanation=msg) + + return value + def update(self, req, body, package_id): """List of allowed changes: { "op": "add", "path": "/tags", "value": [ "foo", "bar" ] } @@ -158,28 +176,11 @@ class Controller(object): return package.to_dict() def search(self, req): - def _validate_limit(value): - if value is None: - return - try: - value = int(value) - except ValueError: - msg = _("limit param must be an integer") - LOG.error(msg) - raise exc.HTTPBadRequest(explanation=msg) - - if value <= 0: - msg = _("limit param must be positive") - LOG.error(msg) - raise exc.HTTPBadRequest(explanation=msg) - - return value - policy.check("get_package", req.context) filters = _get_filters(req.GET.items()) - limit = _validate_limit(filters.get('limit')) + limit = self._validate_limit(filters.get('limit')) if limit is None: limit = CONF.packages_opts.limit_param_default limit = min(CONF.packages_opts.api_limit_max, limit) @@ -287,15 +288,65 @@ class Controller(object): category = db_api.category_get(category_id, packages=True) return category.to_dict() + @versionutils.deprecated(as_of=versionutils.deprecated.LIBERTY, + in_favor_of='categories.list()') def show_categories(self, req): policy.check("get_category", req.context) categories = db_api.categories_list() return {'categories': [category.name for category in categories]} def list_categories(self, req): + """List all categories with pagination and sorting + Acceptable filter params: + :param sort_keys: an array of fields used to sort the list + :param sort_dir: the direction of the sort ('asc' or 'desc') + :param limit: the number of categories to list + :param marker: the ID of the last item in the previous page + """ + def _get_category_filters(req): + query_params = {} + valid_query_params = ['sort_keys', 'sort_dir', 'limit', 'marker'] + for key, value in req.GET.items(): + if key not in valid_query_params: + raise exc.HTTPBadRequest( + _('Bad value passed to filter.' + ' Got {key}, exected:{valid}').format( + key=key, valid=', '.join(valid_query_params))) + if key == 'sort_keys': + available_sort_keys = ['name', 'created', + 'updated', 'package_count', 'id'] + value = [v.strip() for v in value.split(',')] + for sort_key in value: + if sort_key not in available_sort_keys: + raise exc.HTTPBadRequest( + explanation=_('Invalid sort key: {sort_key}.' + ' Must be one of the following:' + ' {available}').format( + sort_key=sort_key, + available=', '.join(available_sort_keys))) + if key == 'sort_dir': + if value not in ['asc', 'desc']: + msg = _('Invalid sort direction: {0}').format(value) + raise exc.HTTPBadRequest(explanation=msg) + query_params[key] = value + return query_params + policy.check("get_category", req.context) - categories = db_api.categories_list() - return {'categories': [category.to_dict() for category in categories]} + + filters = _get_category_filters(req) + + marker = filters.get('marker') + limit = self._validate_limit(filters.get('limit')) + + result = {} + categories = db_api.categories_list(filters, + limit=limit, + marker=marker) + if len(categories) == limit: + result['next_marker'] = categories[-1].id + + result['categories'] = [category.to_dict() for category in categories] + return result def add_category(self, req, body=None): policy.check("add_category", req.context) diff --git a/murano/db/catalog/api.py b/murano/db/catalog/api.py index 4d6a4977..1d560018 100644 --- a/murano/db/catalog/api.py +++ b/murano/db/catalog/api.py @@ -413,9 +413,20 @@ def category_get(category_id, session=None, packages=False): return category -def categories_list(): +def categories_list(filters=None, limit=None, marker=None): + if filters is None: + filters = {} + sort_keys = filters.get('sort_keys', ['name']) + sort_dir = filters.get('sort_dir', 'asc') + session = db_session.get_session() - return session.query(models.Category).all() + query = session.query(models.Category) + if marker is not None: + marker = category_get(marker, session) + + query = utils.paginate_query( + query, models.Category, limit, sort_keys, marker, sort_dir) + return query.all() def category_get_names(): diff --git a/murano/tests/unit/api/v1/test_catalog.py b/murano/tests/unit/api/v1/test_catalog.py index 2aff68a9..bd87c934 100644 --- a/murano/tests/unit/api/v1/test_catalog.py +++ b/murano/tests/unit/api/v1/test_catalog.py @@ -406,3 +406,54 @@ This is a fake zip archive result_message = result.text.replace('\n', '') self.assertIn('Category name should be 80 characters maximum', result_message) + + def test_list_category(self): + names = ['cat1', 'cat2'] + for name in names: + db_catalog_api.category_add(name) + + self._set_policy_rules({'get_category': '@'}) + self.expect_policy_check('get_category') + + req = self._get('/catalog/categories') + result = req.get_response(self.api) + self.assertEqual(200, result.status_code) + result_categories = json.loads(result.body)['categories'] + self.assertEqual(2, len(result_categories)) + self.assertEqual(names, [c['name'] for c in result_categories]) + + params = {'sort_keys': 'created, id'} + req = self._get('/catalog/categories', params) + self.expect_policy_check('get_category') + result = req.get_response(self.api) + self.assertEqual(200, result.status_code) + result_categories = json.loads(result.body)['categories'] + self.assertEqual(names, [c['name'] for c in result_categories]) + + names.reverse() + + params = {'sort_dir': 'desc'} + req = self._get('/catalog/categories', params) + self.expect_policy_check('get_category') + result = req.get_response(self.api) + self.assertEqual(200, result.status_code) + result_categories = json.loads(result.body)['categories'] + self.assertEqual(names, [c['name'] for c in result_categories]) + + def test_list_category_negative(self): + self._set_policy_rules({'get_category': '@'}) + self.expect_policy_check('get_category') + + req = self._get('/catalog/categories', {'sort_dir': 'test'}) + result = req.get_response(self.api) + self.assertEqual(400, result.status_code) + + self.expect_policy_check('get_category') + req = self._get('/catalog/categories', {'sort_keys': 'test'}) + result = req.get_response(self.api) + self.assertEqual(400, result.status_code) + + self.expect_policy_check('get_category') + req = self._get('/catalog/categories', {'test': ['test']}) + result = req.get_response(self.api) + self.assertEqual(400, result.status_code) diff --git a/murano/tests/unit/db/test_catalog.py b/murano/tests/unit/db/test_catalog.py index 2d2d6dab..a26b24ce 100644 --- a/murano/tests/unit/db/test_catalog.py +++ b/murano/tests/unit/db/test_catalog.py @@ -497,3 +497,27 @@ class CatalogDBTestCase(base.MuranoWithDBTestCase): api.package_update(id1, [patch], self.context) self.assertRaises(exc.HTTPConflict, api.package_update, id2, [patch], self.context_2) + + def test_category_paginate(self): + """Paginate through a list of categories using limit and marker""" + + category_names = ['cat1', 'cat2', 'cat3', 'cat4', 'cat5'] + categories = [] + for name in category_names: + categories.append(api.category_add(name)) + uuids = [c.id for c in categories] + + page = api.categories_list(limit=2) + + self.assertEqual(category_names[:2], [c.name for c in page]) + + last = page[-1].id + page = api.categories_list(limit=3, marker=last) + self.assertEqual(category_names[2:5], [c.name for c in page]) + + page = api.categories_list(marker=uuids[-1]) + self.assertEqual([], page) + + category_names.reverse() + page = api.categories_list({'sort_dir': 'desc'}) + self.assertEqual(category_names, [c.name for c in page])