From b6a47992e7299dbb4ed684ecaf05d35d669c38b8 Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Fri, 13 Jan 2023 10:36:32 +0000 Subject: [PATCH] Fix sorting error when type is different There was a logical flaw where we compared types that were different causing a TypeError. There was also a flaw in that we default to the name of the action when sorting if the key does not exist, to compare the data both value should come from the same key as well otherwise they should be treated as not equal since the data cannot be. Change-Id: Idcb276912582bb097dc5c1c9692facde63d5886b --- mistral/api/controllers/v2/action.py | 9 ++++++--- mistral/tests/unit/api/v2/test_actions.py | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/mistral/api/controllers/v2/action.py b/mistral/api/controllers/v2/action.py index 11d79aede..04949d83e 100644 --- a/mistral/api/controllers/v2/action.py +++ b/mistral/api/controllers/v2/action.py @@ -330,8 +330,8 @@ class ActionsController(rest.RestController, hooks.HookController): def action_descriptor_sort(a_ds, keys, dirs): def compare_(a_d1, a_d2): for key, dir in zip(keys, dirs): - a_d1 = getattr(a_d1, key, a_d1.name) - a_d2 = getattr(a_d2, key, a_d2.name) + a_d1 = getattr(a_d1, key, None) + a_d2 = getattr(a_d2, key, None) if a_d1 is None and a_d2 is None: ret = 0 @@ -340,7 +340,10 @@ class ActionsController(rest.RestController, hooks.HookController): elif a_d1 is not None and a_d2 is None: ret = 1 else: - ret = (a_d1 > a_d2) - (a_d1 < a_d2) + if type(a_d1) is type(a_d2): + ret = (a_d1 > a_d2) - (a_d1 < a_d2) + else: + ret = 1 if ret: return ret * (1 if dir == 'asc' else -1) return 0 diff --git a/mistral/tests/unit/api/v2/test_actions.py b/mistral/tests/unit/api/v2/test_actions.py index b2df98b50..a07f7d7a1 100644 --- a/mistral/tests/unit/api/v2/test_actions.py +++ b/mistral/tests/unit/api/v2/test_actions.py @@ -452,7 +452,8 @@ class TestActionsController(base.APITest): # Create an adhoc action for the purpose of the test. adhoc_actions.create_actions(ADHOC_ACTION_YAML) - resp = self.app.get('/v2/actions?limit=1&sort_keys=id,name') + resp = self.app.get( + '/v2/actions?limit=1&sort_keys=created_at&sort_dirs=desc') self.assertEqual(200, resp.status_int) self.assertIn('next', resp.json) @@ -472,14 +473,25 @@ class TestActionsController(base.APITest): expected_dict = { 'marker': action_def.id, 'limit': 1, - 'sort_keys': 'id,name', - 'sort_dirs': 'asc,asc' + 'sort_keys': 'created_at,id', + 'sort_dirs': 'desc,asc' } self.assertTrue( set(expected_dict.items()).issubset(set(param_dict.items())) ) + def test_get_all_sort_date_asc(self): + adhoc_actions.create_actions(ADHOC_ACTION_YAML) + resp = self.app.get('/v2/actions?sort_keys=created_at&sort_dirs=asc') + self.assertEqual(200, resp.status_int) + + def test_get_all_sort_date_desc(self): + adhoc_actions.create_actions(ADHOC_ACTION_YAML_A) + adhoc_actions.create_actions(ADHOC_ACTION_YAML_B) + resp = self.app.get('/v2/actions?sort_keys=created_at&sort_dirs=desc') + self.assertEqual(200, resp.status_int) + def test_get_all_pagination_marker(self): adhoc_actions.create_actions(ADHOC_ACTION_YAML_B) adhoc_actions.create_actions(ADHOC_ACTION_YAML_A)