From 3075723da91eebe75638dc6b5b09d0aabfed1410 Mon Sep 17 00:00:00 2001 From: Egor Panfilov Date: Tue, 13 Mar 2018 17:24:50 +0300 Subject: [PATCH] Fix sort of *list command output While sorting output of list command ("audittemplate list", "strategy list", etc) by sort-key that is not belongs to specific model, this sort-key was passed to db what caused error (HTTP 500). We added check on such keys and now, if got one of them, then we make sort on API side instead of db side. We removed excess sort and changed all sorting routines to unified way. Also added sort tests on every model. Change-Id: I41faea1622605ee4fa8dc48cd572876d75be8383 Closes-Bug: 1662887 --- watcher/api/controllers/v1/action.py | 25 +++++--- watcher/api/controllers/v1/action_plan.py | 37 +++++------- watcher/api/controllers/v1/audit.py | 41 ++++++------- watcher/api/controllers/v1/audit_template.py | 39 +++++++----- watcher/api/controllers/v1/goal.py | 19 ++---- watcher/api/controllers/v1/scoring_engine.py | 17 ++---- watcher/api/controllers/v1/service.py | 19 ++---- watcher/api/controllers/v1/strategy.py | 34 +++++------ watcher/api/controllers/v1/utils.py | 22 +++++++ watcher/tests/api/v1/test_actions.py | 62 ++++++++++++++++++++ watcher/tests/api/v1/test_actions_plans.py | 6 ++ watcher/tests/api/v1/test_audit_templates.py | 44 ++++++++++++++ watcher/tests/api/v1/test_audits.py | 6 ++ watcher/tests/api/v1/test_goals.py | 21 +++++++ watcher/tests/api/v1/test_scoring_engines.py | 20 +++++++ watcher/tests/api/v1/test_services.py | 20 +++++++ watcher/tests/api/v1/test_strategies.py | 21 +++++++ 17 files changed, 331 insertions(+), 122 deletions(-) diff --git a/watcher/api/controllers/v1/action.py b/watcher/api/controllers/v1/action.py index 113ad8bed..baadf418c 100644 --- a/watcher/api/controllers/v1/action.py +++ b/watcher/api/controllers/v1/action.py @@ -205,7 +205,7 @@ class ActionCollection(collection.Collection): collection = ActionCollection() collection.actions = [Action.convert_with_links(p, expand) for p in actions] - + collection.next = collection.get_next(limit, url=url, **kwargs) return collection @classmethod @@ -232,6 +232,10 @@ class ActionsController(rest.RestController): sort_key, sort_dir, expand=False, resource_url=None, action_plan_uuid=None, audit_uuid=None): + additional_fields = ['action_plan_uuid'] + + api_utils.validate_sort_key(sort_key, list(objects.Action.fields) + + additional_fields) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) @@ -247,7 +251,10 @@ class ActionsController(rest.RestController): if audit_uuid: filters['audit_uuid'] = audit_uuid - sort_db_key = sort_key + need_api_sort = api_utils.check_need_api_sort(sort_key, + additional_fields) + sort_db_key = (sort_key if not need_api_sort + else None) actions = objects.Action.list(pecan.request.context, limit, @@ -255,11 +262,15 @@ class ActionsController(rest.RestController): sort_dir=sort_dir, filters=filters) - return ActionCollection.convert_with_links(actions, limit, - url=resource_url, - expand=expand, - sort_key=sort_key, - sort_dir=sort_dir) + actions_collection = ActionCollection.convert_with_links( + actions, limit, url=resource_url, expand=expand, + sort_key=sort_key, sort_dir=sort_dir) + + if need_api_sort: + api_utils.make_api_sort(actions_collection.actions, + sort_key, sort_dir) + + return actions_collection @wsme_pecan.wsexpose(ActionCollection, types.uuid, int, wtypes.text, wtypes.text, types.uuid, diff --git a/watcher/api/controllers/v1/action_plan.py b/watcher/api/controllers/v1/action_plan.py index 46e79f443..ee46f85d4 100644 --- a/watcher/api/controllers/v1/action_plan.py +++ b/watcher/api/controllers/v1/action_plan.py @@ -305,17 +305,6 @@ class ActionPlanCollection(collection.Collection): ap_collection = ActionPlanCollection() ap_collection.action_plans = [ActionPlan.convert_with_links( p, expand) for p in rpc_action_plans] - - if 'sort_key' in kwargs: - reverse = False - if kwargs['sort_key'] == 'audit_uuid': - if 'sort_dir' in kwargs: - reverse = True if kwargs['sort_dir'] == 'desc' else False - ap_collection.action_plans = sorted( - ap_collection.action_plans, - key=lambda action_plan: action_plan.audit_uuid, - reverse=reverse) - ap_collection.next = ap_collection.get_next(limit, url=url, **kwargs) return ap_collection @@ -344,7 +333,10 @@ class ActionPlansController(rest.RestController): sort_key, sort_dir, expand=False, resource_url=None, audit_uuid=None, strategy=None): + additional_fields = ['audit_uuid', 'strategy_uuid', 'strategy_name'] + api_utils.validate_sort_key( + sort_key, list(objects.ActionPlan.fields) + additional_fields) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) @@ -363,10 +355,10 @@ class ActionPlansController(rest.RestController): else: filters['strategy_name'] = strategy - if sort_key == 'audit_uuid': - sort_db_key = None - else: - sort_db_key = sort_key + need_api_sort = api_utils.check_need_api_sort(sort_key, + additional_fields) + sort_db_key = (sort_key if not need_api_sort + else None) action_plans = objects.ActionPlan.list( pecan.request.context, @@ -374,12 +366,15 @@ class ActionPlansController(rest.RestController): marker_obj, sort_key=sort_db_key, sort_dir=sort_dir, filters=filters) - return ActionPlanCollection.convert_with_links( - action_plans, limit, - url=resource_url, - expand=expand, - sort_key=sort_key, - sort_dir=sort_dir) + action_plans_collection = ActionPlanCollection.convert_with_links( + action_plans, limit, url=resource_url, expand=expand, + sort_key=sort_key, sort_dir=sort_dir) + + if need_api_sort: + api_utils.make_api_sort(action_plans_collection.action_plans, + sort_key, sort_dir) + + return action_plans_collection @wsme_pecan.wsexpose(ActionPlanCollection, types.uuid, int, wtypes.text, wtypes.text, types.uuid, wtypes.text) diff --git a/watcher/api/controllers/v1/audit.py b/watcher/api/controllers/v1/audit.py index c72b382a5..81fa97319 100644 --- a/watcher/api/controllers/v1/audit.py +++ b/watcher/api/controllers/v1/audit.py @@ -389,17 +389,6 @@ class AuditCollection(collection.Collection): collection = AuditCollection() collection.audits = [Audit.convert_with_links(p, expand) for p in rpc_audits] - - if 'sort_key' in kwargs: - reverse = False - if kwargs['sort_key'] == 'goal_uuid': - if 'sort_dir' in kwargs: - reverse = True if kwargs['sort_dir'] == 'desc' else False - collection.audits = sorted( - collection.audits, - key=lambda audit: audit.goal_uuid, - reverse=reverse) - collection.next = collection.get_next(limit, url=url, **kwargs) return collection @@ -427,8 +416,14 @@ class AuditsController(rest.RestController): sort_key, sort_dir, expand=False, resource_url=None, goal=None, strategy=None): + additional_fields = ["goal_uuid", "goal_name", "strategy_uuid", + "strategy_name"] + + api_utils.validate_sort_key( + sort_key, list(objects.Audit.fields) + additional_fields) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) + marker_obj = None if marker: marker_obj = objects.Audit.get_by_uuid(pecan.request.context, @@ -449,23 +444,25 @@ class AuditsController(rest.RestController): # TODO(michaelgugino): add method to get goal by name. filters['strategy_name'] = strategy - if sort_key == 'goal_uuid': - sort_db_key = 'goal_id' - elif sort_key == 'strategy_uuid': - sort_db_key = 'strategy_id' - else: - sort_db_key = sort_key + need_api_sort = api_utils.check_need_api_sort(sort_key, + additional_fields) + sort_db_key = (sort_key if not need_api_sort + else None) audits = objects.Audit.list(pecan.request.context, limit, marker_obj, sort_key=sort_db_key, sort_dir=sort_dir, filters=filters) - return AuditCollection.convert_with_links(audits, limit, - url=resource_url, - expand=expand, - sort_key=sort_key, - sort_dir=sort_dir) + audits_collection = AuditCollection.convert_with_links( + audits, limit, url=resource_url, expand=expand, + sort_key=sort_key, sort_dir=sort_dir) + + if need_api_sort: + api_utils.make_api_sort(audits_collection.audits, sort_key, + sort_dir) + + return audits_collection @wsme_pecan.wsexpose(AuditCollection, types.uuid, int, wtypes.text, wtypes.text, wtypes.text, wtypes.text, int) diff --git a/watcher/api/controllers/v1/audit_template.py b/watcher/api/controllers/v1/audit_template.py index ac967599c..8f4c3b373 100644 --- a/watcher/api/controllers/v1/audit_template.py +++ b/watcher/api/controllers/v1/audit_template.py @@ -474,9 +474,13 @@ class AuditTemplatesController(rest.RestController): def _get_audit_templates_collection(self, filters, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): + additional_fields = ["goal_uuid", "goal_name", "strategy_uuid", + "strategy_name"] + + api_utils.validate_sort_key( + sort_key, list(objects.AuditTemplate.fields) + additional_fields) api_utils.validate_search_filters( - filters, list(objects.audit_template.AuditTemplate.fields) + - ["goal_uuid", "goal_name", "strategy_uuid", "strategy_name"]) + filters, list(objects.AuditTemplate.fields) + additional_fields) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) @@ -486,19 +490,26 @@ class AuditTemplatesController(rest.RestController): pecan.request.context, marker) - audit_templates = objects.AuditTemplate.list( - pecan.request.context, - filters, - limit, - marker_obj, sort_key=sort_key, - sort_dir=sort_dir) + need_api_sort = api_utils.check_need_api_sort(sort_key, + additional_fields) + sort_db_key = (sort_key if not need_api_sort + else None) - return AuditTemplateCollection.convert_with_links(audit_templates, - limit, - url=resource_url, - expand=expand, - sort_key=sort_key, - sort_dir=sort_dir) + audit_templates = objects.AuditTemplate.list( + pecan.request.context, filters, limit, marker_obj, + sort_key=sort_db_key, sort_dir=sort_dir) + + audit_templates_collection = \ + AuditTemplateCollection.convert_with_links( + audit_templates, limit, url=resource_url, expand=expand, + sort_key=sort_key, sort_dir=sort_dir) + + if need_api_sort: + api_utils.make_api_sort( + audit_templates_collection.audit_templates, sort_key, + sort_dir) + + return audit_templates_collection @wsme_pecan.wsexpose(AuditTemplateCollection, wtypes.text, wtypes.text, types.uuid, int, wtypes.text, wtypes.text) diff --git a/watcher/api/controllers/v1/goal.py b/watcher/api/controllers/v1/goal.py index 43ba33d29..8d98d7c63 100644 --- a/watcher/api/controllers/v1/goal.py +++ b/watcher/api/controllers/v1/goal.py @@ -130,17 +130,6 @@ class GoalCollection(collection.Collection): goal_collection = GoalCollection() goal_collection.goals = [ Goal.convert_with_links(g, expand) for g in goals] - - if 'sort_key' in kwargs: - reverse = False - if kwargs['sort_key'] == 'strategy': - if 'sort_dir' in kwargs: - reverse = True if kwargs['sort_dir'] == 'desc' else False - goal_collection.goals = sorted( - goal_collection.goals, - key=lambda goal: goal.uuid, - reverse=reverse) - goal_collection.next = goal_collection.get_next( limit, url=url, **kwargs) return goal_collection @@ -167,17 +156,19 @@ class GoalsController(rest.RestController): def _get_goals_collection(self, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): + api_utils.validate_sort_key( + sort_key, list(objects.Goal.fields)) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) - sort_db_key = (sort_key if sort_key in objects.Goal.fields - else None) - marker_obj = None if marker: marker_obj = objects.Goal.get_by_uuid( pecan.request.context, marker) + sort_db_key = (sort_key if sort_key in objects.Goal.fields + else None) + goals = objects.Goal.list(pecan.request.context, limit, marker_obj, sort_key=sort_db_key, sort_dir=sort_dir) diff --git a/watcher/api/controllers/v1/scoring_engine.py b/watcher/api/controllers/v1/scoring_engine.py index 0e13d3831..d1b98fefb 100644 --- a/watcher/api/controllers/v1/scoring_engine.py +++ b/watcher/api/controllers/v1/scoring_engine.py @@ -123,17 +123,6 @@ class ScoringEngineCollection(collection.Collection): collection = ScoringEngineCollection() collection.scoring_engines = [ScoringEngine.convert_with_links( se, expand) for se in scoring_engines] - - if 'sort_key' in kwargs: - reverse = False - if kwargs['sort_key'] == 'name': - if 'sort_dir' in kwargs: - reverse = True if kwargs['sort_dir'] == 'desc' else False - collection.goals = sorted( - collection.scoring_engines, - key=lambda se: se.name, - reverse=reverse) - collection.next = collection.get_next(limit, url=url, **kwargs) return collection @@ -160,7 +149,8 @@ class ScoringEngineController(rest.RestController): def _get_scoring_engines_collection(self, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): - + api_utils.validate_sort_key( + sort_key, list(objects.ScoringEngine.fields)) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) @@ -171,7 +161,8 @@ class ScoringEngineController(rest.RestController): filters = {} - sort_db_key = sort_key + sort_db_key = (sort_key if sort_key in objects.ScoringEngine.fields + else None) scoring_engines = objects.ScoringEngine.list( context=pecan.request.context, diff --git a/watcher/api/controllers/v1/service.py b/watcher/api/controllers/v1/service.py index e7f77c93f..734975e17 100644 --- a/watcher/api/controllers/v1/service.py +++ b/watcher/api/controllers/v1/service.py @@ -154,17 +154,6 @@ class ServiceCollection(collection.Collection): service_collection = ServiceCollection() service_collection.services = [ Service.convert_with_links(g, expand) for g in services] - - if 'sort_key' in kwargs: - reverse = False - if kwargs['sort_key'] == 'service': - if 'sort_dir' in kwargs: - reverse = True if kwargs['sort_dir'] == 'desc' else False - service_collection.services = sorted( - service_collection.services, - key=lambda service: service.id, - reverse=reverse) - service_collection.next = service_collection.get_next( limit, url=url, marker_field='id', **kwargs) return service_collection @@ -191,17 +180,19 @@ class ServicesController(rest.RestController): def _get_services_collection(self, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): + api_utils.validate_sort_key( + sort_key, list(objects.Service.fields)) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) - sort_db_key = (sort_key if sort_key in objects.Service.fields - else None) - marker_obj = None if marker: marker_obj = objects.Service.get( pecan.request.context, marker) + sort_db_key = (sort_key if sort_key in objects.Service.fields + else None) + services = objects.Service.list( pecan.request.context, limit, marker_obj, sort_key=sort_db_key, sort_dir=sort_dir) diff --git a/watcher/api/controllers/v1/strategy.py b/watcher/api/controllers/v1/strategy.py index 850cd3183..32f853363 100644 --- a/watcher/api/controllers/v1/strategy.py +++ b/watcher/api/controllers/v1/strategy.py @@ -173,17 +173,6 @@ class StrategyCollection(collection.Collection): strategy_collection = StrategyCollection() strategy_collection.strategies = [ Strategy.convert_with_links(g, expand) for g in strategies] - - if 'sort_key' in kwargs: - reverse = False - if kwargs['sort_key'] == 'strategy': - if 'sort_dir' in kwargs: - reverse = True if kwargs['sort_dir'] == 'desc' else False - strategy_collection.strategies = sorted( - strategy_collection.strategies, - key=lambda strategy: strategy.uuid, - reverse=reverse) - strategy_collection.next = strategy_collection.get_next( limit, url=url, **kwargs) return strategy_collection @@ -211,28 +200,39 @@ class StrategiesController(rest.RestController): def _get_strategies_collection(self, filters, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): + additional_fields = ["goal_uuid", "goal_name"] + + api_utils.validate_sort_key( + sort_key, list(objects.Strategy.fields) + additional_fields) api_utils.validate_search_filters( - filters, list(objects.strategy.Strategy.fields) + - ["goal_uuid", "goal_name"]) + filters, list(objects.Strategy.fields) + additional_fields) limit = api_utils.validate_limit(limit) api_utils.validate_sort_dir(sort_dir) - sort_db_key = (sort_key if sort_key in objects.Strategy.fields - else None) - marker_obj = None if marker: marker_obj = objects.Strategy.get_by_uuid( pecan.request.context, marker) + need_api_sort = api_utils.check_need_api_sort(sort_key, + additional_fields) + sort_db_key = (sort_key if not need_api_sort + else None) + strategies = objects.Strategy.list( pecan.request.context, limit, marker_obj, filters=filters, sort_key=sort_db_key, sort_dir=sort_dir) - return StrategyCollection.convert_with_links( + strategies_collection = StrategyCollection.convert_with_links( strategies, limit, url=resource_url, expand=expand, sort_key=sort_key, sort_dir=sort_dir) + if need_api_sort: + api_utils.make_api_sort(strategies_collection.strategies, + sort_key, sort_dir) + + return strategies_collection + @wsme_pecan.wsexpose(StrategyCollection, wtypes.text, wtypes.text, int, wtypes.text, wtypes.text) def get_all(self, goal=None, marker=None, limit=None, diff --git a/watcher/api/controllers/v1/utils.py b/watcher/api/controllers/v1/utils.py index 60d503829..956edbfed 100644 --- a/watcher/api/controllers/v1/utils.py +++ b/watcher/api/controllers/v1/utils.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +from operator import attrgetter + import jsonpatch from oslo_config import cfg from oslo_utils import reflection @@ -54,6 +56,13 @@ def validate_sort_dir(sort_dir): "'asc' or 'desc'") % sort_dir) +def validate_sort_key(sort_key, allowed_fields): + # Very lightweight validation for now + if sort_key not in allowed_fields: + raise wsme.exc.ClientSideError( + _("Invalid sort key: %s") % sort_key) + + def validate_search_filters(filters, allowed_fields): # Very lightweight validation for now # todo: improve this (e.g. https://www.parse.com/docs/rest/guide/#queries) @@ -63,6 +72,19 @@ def validate_search_filters(filters, allowed_fields): _("Invalid filter: %s") % filter_name) +def check_need_api_sort(sort_key, additional_fields): + return sort_key in additional_fields + + +def make_api_sort(sorting_list, sort_key, sort_dir): + # First sort by uuid field, than sort by sort_key + # sort() ensures stable sorting, so we could + # make lexicographical sort + reverse_direction = (sort_dir == 'desc') + sorting_list.sort(key=attrgetter('uuid'), reverse=reverse_direction) + sorting_list.sort(key=attrgetter(sort_key), reverse=reverse_direction) + + def apply_jsonpatch(doc, patch): for p in patch: if p['op'] == 'add' and p['path'].count('/') == 1: diff --git a/watcher/tests/api/v1/test_actions.py b/watcher/tests/api/v1/test_actions.py index b2246f6b4..60d6bd0e9 100644 --- a/watcher/tests/api/v1/test_actions.py +++ b/watcher/tests/api/v1/test_actions.py @@ -11,6 +11,7 @@ # limitations under the License. import datetime +import itertools import mock from oslo_config import cfg @@ -267,6 +268,67 @@ class TestListAction(api_base.FunctionalTest): response = self.get_json(url, expect_errors=True) self.assertEqual(400, response.status_int) + def test_many_with_sort_key_uuid(self): + action_plan = obj_utils.create_test_action_plan( + self.context, + uuid=utils.generate_uuid(), + audit_id=self.audit.id) + + actions_list = [] + for id_ in range(1, 3): + action = obj_utils.create_test_action( + self.context, id=id_, + action_plan_id=action_plan.id, + uuid=utils.generate_uuid()) + actions_list.append(action) + + response = self.get_json('/actions?sort_key=%s' % 'uuid') + names = [s['uuid'] for s in response['actions']] + + self.assertEqual( + sorted([a.uuid for a in actions_list]), + names) + + def test_many_with_sort_key_action_plan_uuid(self): + action_plan_1 = obj_utils.create_test_action_plan( + self.context, + uuid=utils.generate_uuid(), + audit_id=self.audit.id) + + action_plan_2 = obj_utils.create_test_action_plan( + self.context, + uuid=utils.generate_uuid(), + audit_id=self.audit.id) + + action_plans_uuid_list = [] + for id_, action_plan_id in enumerate(itertools.chain.from_iterable([ + itertools.repeat(action_plan_1.id, 3), + itertools.repeat(action_plan_2.id, 2)]), 1): + action = obj_utils.create_test_action( + self.context, id=id_, + action_plan_id=action_plan_id, + uuid=utils.generate_uuid()) + action_plans_uuid_list.append(action.action_plan.uuid) + + for direction in ['asc', 'desc']: + response = self.get_json( + '/actions?sort_key={0}&sort_dir={1}' + .format('action_plan_uuid', direction)) + + action_plan_uuids = \ + [s['action_plan_uuid'] for s in response['actions']] + + self.assertEqual( + sorted(action_plans_uuid_list, reverse=(direction == 'desc')), + action_plan_uuids, + message='Failed on %s direction' % direction) + + def test_sort_key_validation(self): + response = self.get_json( + '/actions?sort_key=%s' % 'bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + def test_many_with_soft_deleted_action_plan_uuid(self): action_plan1 = obj_utils.create_test_action_plan( self.context, diff --git a/watcher/tests/api/v1/test_actions_plans.py b/watcher/tests/api/v1/test_actions_plans.py index a001a99d9..5234b71f3 100644 --- a/watcher/tests/api/v1/test_actions_plans.py +++ b/watcher/tests/api/v1/test_actions_plans.py @@ -256,6 +256,12 @@ class TestListActionPlan(api_base.FunctionalTest): uuids = [s['audit_uuid'] for s in response['action_plans']] self.assertEqual(sorted(audit_list), uuids) + def test_sort_key_validation(self): + response = self.get_json( + '/action_plans?sort_key=%s' % 'bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + def test_links(self): uuid = utils.generate_uuid() obj_utils.create_test_action_plan(self.context, id=1, uuid=uuid) diff --git a/watcher/tests/api/v1/test_audit_templates.py b/watcher/tests/api/v1/test_audit_templates.py index 0690f6cb4..dc990e4d2 100644 --- a/watcher/tests/api/v1/test_audit_templates.py +++ b/watcher/tests/api/v1/test_audit_templates.py @@ -294,6 +294,50 @@ class TestListAuditTemplate(FunctionalTestWithSetup): '/audit_templates?strategy=%s' % self.fake_strategy2.name) self.assertEqual(2, len(response['audit_templates'])) + def test_many_with_sort_key_name(self): + audit_template_list = [] + for id_ in range(1, 6): + audit_template = obj_utils.create_test_audit_template( + self.context, id=id_, uuid=utils.generate_uuid(), + name='My Audit Template {0}'.format(id_)) + audit_template_list.append(audit_template) + + response = self.get_json('/audit_templates?sort_key=%s' % 'name') + + names = [s['name'] for s in response['audit_templates']] + + self.assertEqual( + sorted([at.name for at in audit_template_list]), + names) + + def test_many_with_sort_key_goal_name(self): + goal_names_list = [] + for id_, goal_id in enumerate(itertools.chain.from_iterable([ + itertools.repeat(self.fake_goal1.id, 3), + itertools.repeat(self.fake_goal2.id, 2)]), 1): + audit_template = obj_utils.create_test_audit_template( + self.context, id=id_, uuid=utils.generate_uuid(), + name='My Audit Template {0}'.format(id_), + goal_id=goal_id) + goal_names_list.append(audit_template.goal.name) + + for direction in ['asc', 'desc']: + response = self.get_json( + '/audit_templates?sort_key={0}&sort_dir={1}' + .format('goal_name', direction)) + + goal_names = [s['goal_name'] for s in response['audit_templates']] + + self.assertEqual( + sorted(goal_names_list, reverse=(direction == 'desc')), + goal_names) + + def test_sort_key_validation(self): + response = self.get_json( + '/audit_templates?sort_key=%s' % 'goal_bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + class TestPatch(FunctionalTestWithSetup): diff --git a/watcher/tests/api/v1/test_audits.py b/watcher/tests/api/v1/test_audits.py index a82366d3a..79d9d6aab 100644 --- a/watcher/tests/api/v1/test_audits.py +++ b/watcher/tests/api/v1/test_audits.py @@ -217,6 +217,12 @@ class TestListAudit(api_base.FunctionalTest): uuids = [s['goal_uuid'] for s in response['audits']] self.assertEqual(sorted(goal_list), uuids) + def test_sort_key_validation(self): + response = self.get_json( + '/audits?sort_key=%s' % 'bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + def test_links(self): uuid = utils.generate_uuid() obj_utils.create_test_audit( diff --git a/watcher/tests/api/v1/test_goals.py b/watcher/tests/api/v1/test_goals.py index 6c71c1598..2d9bd0afb 100644 --- a/watcher/tests/api/v1/test_goals.py +++ b/watcher/tests/api/v1/test_goals.py @@ -120,6 +120,27 @@ class TestListGoal(api_base.FunctionalTest): response = self.get_json('/goals') self.assertEqual(3, len(response['goals'])) + def test_many_with_sort_key_uuid(self): + goal_list = [] + for idx in range(1, 6): + goal = obj_utils.create_test_goal( + self.context, id=idx, + uuid=utils.generate_uuid(), + name='GOAL_{0}'.format(idx)) + goal_list.append(goal.uuid) + + response = self.get_json('/goals/?sort_key=uuid') + + self.assertEqual(5, len(response['goals'])) + uuids = [s['uuid'] for s in response['goals']] + self.assertEqual(sorted(goal_list), uuids) + + def test_sort_key_validation(self): + response = self.get_json( + '/goals?sort_key=%s' % 'bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + class TestGoalPolicyEnforcement(api_base.FunctionalTest): diff --git a/watcher/tests/api/v1/test_scoring_engines.py b/watcher/tests/api/v1/test_scoring_engines.py index 2e7b3cc2c..bbf7cb706 100644 --- a/watcher/tests/api/v1/test_scoring_engines.py +++ b/watcher/tests/api/v1/test_scoring_engines.py @@ -113,6 +113,26 @@ class TestListScoringEngine(api_base.FunctionalTest): response = self.get_json('/scoring_engines') self.assertEqual(3, len(response['scoring_engines'])) + def test_many_with_sort_key_uuid(self): + scoring_engine_list = [] + for idx in range(1, 6): + scoring_engine = obj_utils.create_test_scoring_engine( + self.context, id=idx, uuid=utils.generate_uuid(), + name=str(idx), description='SE_{0}'.format(idx)) + scoring_engine_list.append(scoring_engine.uuid) + + response = self.get_json('/scoring_engines/?sort_key=uuid') + + self.assertEqual(5, len(response['scoring_engines'])) + uuids = [s['uuid'] for s in response['scoring_engines']] + self.assertEqual(sorted(scoring_engine_list), uuids) + + def test_sort_key_validation(self): + response = self.get_json( + '/goals?sort_key=%s' % 'bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + class TestScoringEnginePolicyEnforcement(api_base.FunctionalTest): diff --git a/watcher/tests/api/v1/test_services.py b/watcher/tests/api/v1/test_services.py index c556d9597..8bc7f3581 100644 --- a/watcher/tests/api/v1/test_services.py +++ b/watcher/tests/api/v1/test_services.py @@ -131,6 +131,26 @@ class TestListService(api_base.FunctionalTest): response = self.get_json('/services') self.assertEqual(3, len(response['services'])) + def test_many_with_sort_key_name(self): + service_list = [] + for id_ in range(1, 4): + service = obj_utils.create_test_service( + self.context, id=id_, host='CONTROLLER', + name='SERVICE_{0}'.format(id_)) + service_list.append(service.name) + + response = self.get_json('/services/?sort_key=name') + + self.assertEqual(3, len(response['services'])) + names = [s['name'] for s in response['services']] + self.assertEqual(sorted(service_list), names) + + def test_sort_key_validation(self): + response = self.get_json( + '/services?sort_key=%s' % 'bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + class TestServicePolicyEnforcement(api_base.FunctionalTest): diff --git a/watcher/tests/api/v1/test_strategies.py b/watcher/tests/api/v1/test_strategies.py index cc4c135ab..abba7becd 100644 --- a/watcher/tests/api/v1/test_strategies.py +++ b/watcher/tests/api/v1/test_strategies.py @@ -221,6 +221,27 @@ class TestListStrategy(api_base.FunctionalTest): for strategy in strategies: self.assertEqual(goal1['uuid'], strategy['goal_uuid']) + def test_many_with_sort_key_goal_uuid(self): + goals_uuid_list = [] + for idx in range(1, 6): + strategy = obj_utils.create_test_strategy( + self.context, id=idx, + uuid=utils.generate_uuid(), + name='STRATEGY_{0}'.format(idx)) + goals_uuid_list.append(strategy.goal.uuid) + + response = self.get_json('/strategies/?sort_key=goal_uuid') + + self.assertEqual(5, len(response['strategies'])) + goal_uuids = [s['goal_uuid'] for s in response['strategies']] + self.assertEqual(sorted(goals_uuid_list), goal_uuids) + + def test_sort_key_validation(self): + response = self.get_json( + '/strategies?sort_key=%s' % 'bad_name', + expect_errors=True) + self.assertEqual(400, response.status_int) + class TestStrategyPolicyEnforcement(api_base.FunctionalTest):