From b99298dfad1611294bf6dfa41712388af78c7197 Mon Sep 17 00:00:00 2001 From: pandatt Date: Fri, 19 Apr 2019 11:29:54 +0800 Subject: [PATCH] Fix bug: AttributeError arises while sorting with standard attributes Common neutron resource(e.g, Port) consists of: 1. Resource Attributes, e.g: Port.mac_address, etc. 2. Standard Attributes, e.g: created_at, and are shared among all neutron resources. The `sort` opt only supports limited attributes. We need to filter attributes that are defined with `is_sort_key=True` and it's preferred to explicitly warn CLI & API users of illegal sort keys rather than just accept without check, pass forward and then hit a internal error which's quite confusing. Depends-on: https://review.opendev.org/#/c/660097/ Change-Id: I8d206f909b09f1279dfcdc25c39989a67bff93d5 Closes-Bug: #1659175 (cherry picked from commit 335ac4e2d95c164d66a3b09d9fd08e9c563edfc2) --- neutron/api/api_common.py | 4 +++- neutron/tests/unit/api/v2/test_base.py | 19 +++++++++++++++++-- ...-check-for-get-sorts-b9e3e86ddcb3bc3a.yaml | 6 ++++++ 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/add-sort-keys-check-for-get-sorts-b9e3e86ddcb3bc3a.yaml diff --git a/neutron/api/api_common.py b/neutron/api/api_common.py index 64db4e9c2ee..f23c8231d0d 100644 --- a/neutron/api/api_common.py +++ b/neutron/api/api_common.py @@ -233,7 +233,9 @@ def get_sorts(request, attr_info): msg = _("The number of sort_keys and sort_dirs must be same") raise exc.HTTPBadRequest(explanation=msg) valid_dirs = [constants.SORT_DIRECTION_ASC, constants.SORT_DIRECTION_DESC] - absent_keys = [x for x in sort_keys if x not in attr_info] + valid_sort_keys = set(attr for attr, schema in attr_info.items() + if schema.get('is_sort_key', False)) + absent_keys = [x for x in sort_keys if x not in valid_sort_keys] if absent_keys: msg = _("%s is invalid attribute for sort_keys") % absent_keys raise exc.HTTPBadRequest(explanation=msg) diff --git a/neutron/tests/unit/api/v2/test_base.py b/neutron/tests/unit/api/v2/test_base.py index 5576eee732c..e003756a906 100644 --- a/neutron/tests/unit/api/v2/test_base.py +++ b/neutron/tests/unit/api/v2/test_base.py @@ -1560,7 +1560,10 @@ class SortingTestCase(base.BaseTestCase): def test_get_sorts(self): path = '/?sort_key=foo&sort_dir=desc&sort_key=bar&sort_dir=asc' request = webob.Request.blank(path) - attr_info = {'foo': {'key': 'val'}, 'bar': {'key': 'val'}} + attr_info = { + 'foo': {'key': 'val', 'is_sort_key': True}, + 'bar': {'key': 'val', 'is_sort_key': True} + } expect_val = [('foo', False), ('bar', True)] actual_val = api_common.get_sorts(request, attr_info) self.assertEqual(expect_val, actual_val) @@ -1568,11 +1571,23 @@ class SortingTestCase(base.BaseTestCase): def test_get_sorts_with_project_id(self): path = '/?sort_key=project_id&sort_dir=desc' request = webob.Request.blank(path) - attr_info = {'tenant_id': {'key': 'val'}} + attr_info = {'tenant_id': {'key': 'val', 'is_sort_key': True}} expect_val = [('project_id', False)] actual_val = api_common.get_sorts(request, attr_info) self.assertEqual(expect_val, actual_val) + def test_get_sorts_with_non_sort_key(self): + path = '/?sort_key=created_at&sort_dir=desc' + request = webob.Request.blank(path) + attr_info = { + 'foo': {'key': 'val', 'is_sort_key': True}, + 'bar': {'key': 'val', 'is_sort_key': True}, + 'created_at': {'key': 'val'} + } + self.assertRaises(exc.HTTPBadRequest, + api_common.get_sorts, + request, attr_info) + class FiltersTestCase(base.BaseTestCase): def test_all_skip_args(self): diff --git a/releasenotes/notes/add-sort-keys-check-for-get-sorts-b9e3e86ddcb3bc3a.yaml b/releasenotes/notes/add-sort-keys-check-for-get-sorts-b9e3e86ddcb3bc3a.yaml new file mode 100644 index 00000000000..0f77727238d --- /dev/null +++ b/releasenotes/notes/add-sort-keys-check-for-get-sorts-b9e3e86ddcb3bc3a.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Add sort-keys validation logic to method ``get_sorts`` in + ``neutron.api.api_common``. See the link below for more: + https://bugs.launchpad.net/neutron/+bug/1659175