Sorting and pagination params used as filters
While retrieving servers, the sort and pagination query string parameters are treated as search options. These parameters are passed down to the DB layer and eventually filtered out when an AttributeError is caught because they do not exist on the Instance model. This change set removes these parameters at the API layer. Closes-Bug: #1446638 Change-Id: I32c92649a2d0346e13c2013bfaa423aaec3b1b67
This commit is contained in:
parent
02174c3552
commit
55f269cf06
|
@ -1153,7 +1153,9 @@ def create_resource(ext_mgr):
|
|||
def remove_invalid_options(context, search_options, allowed_search_options):
|
||||
"""Remove search options that are not valid for non-admin API/context."""
|
||||
if context.is_admin:
|
||||
# Allow all options
|
||||
# Only remove parameters for sorting and pagination
|
||||
for key in ('sort_key', 'sort_dir', 'limit', 'marker'):
|
||||
search_options.pop(key, None)
|
||||
return
|
||||
# Otherwise, strip out all unknown options
|
||||
unknown_options = [opt for opt in search_options
|
||||
|
|
|
@ -1132,7 +1132,9 @@ class ServersController(wsgi.Controller):
|
|||
def remove_invalid_options(context, search_options, allowed_search_options):
|
||||
"""Remove search options that are not valid for non-admin API/context."""
|
||||
if context.is_admin:
|
||||
# Allow all options
|
||||
# Only remove parameters for sorting and pagination
|
||||
for key in ('sort_key', 'sort_dir', 'limit', 'marker'):
|
||||
search_options.pop(key, None)
|
||||
return
|
||||
# Otherwise, strip out all unknown options
|
||||
unknown_options = [opt for opt in search_options
|
||||
|
|
|
@ -1330,6 +1330,19 @@ class ServersControllerTest(ControllerTest):
|
|||
self.assertEqual(s['hostId'], host_ids[i % 2])
|
||||
self.assertEqual(s['name'], 'server%d' % (i + 1))
|
||||
|
||||
@mock.patch.object(compute_api.API, 'get_all')
|
||||
def test_get_servers_remove_non_search_options(self, get_all_mock):
|
||||
req = fakes.HTTPRequest.blank('/fake/servers'
|
||||
'?sort_key=id1&sort_dir=asc'
|
||||
'&sort_key=id2&sort_dir=desc'
|
||||
'&limit=1&marker=123',
|
||||
use_admin_context=True)
|
||||
self.controller.index(req)
|
||||
kwargs = get_all_mock.call_args[1]
|
||||
search_opts = kwargs['search_opts']
|
||||
for key in ('sort_key', 'sort_dir', 'limit', 'marker'):
|
||||
self.assertNotIn(key, search_opts)
|
||||
|
||||
|
||||
class ServersControllerUpdateTest(ControllerTest):
|
||||
|
||||
|
|
|
@ -1400,6 +1400,19 @@ class ServersControllerTestV29(ServersControllerTest):
|
|||
self._test_list_server_detail_with_lock('not_locked',
|
||||
'not_locked')
|
||||
|
||||
@mock.patch.object(compute_api.API, 'get_all')
|
||||
def test_get_servers_remove_non_search_options(self, get_all_mock):
|
||||
req = fakes.HTTPRequestV21.blank('/servers'
|
||||
'?sort_key=id1&sort_dir=asc'
|
||||
'&sort_key=id2&sort_dir=desc'
|
||||
'&limit=1&marker=123',
|
||||
use_admin_context=True)
|
||||
self.controller.index(req)
|
||||
kwargs = get_all_mock.call_args[1]
|
||||
search_opts = kwargs['search_opts']
|
||||
for key in ('sort_key', 'sort_dir', 'limit', 'marker'):
|
||||
self.assertNotIn(key, search_opts)
|
||||
|
||||
|
||||
class ServersControllerDeleteTest(ControllerTest):
|
||||
|
||||
|
|
Loading…
Reference in New Issue