Fix bug for resource routing's filtering feature
1. What is the problem For resource routing Admin API, when we call get_all(self, **kwargs) function with an unsupported filter type specified to list the routing information, the function will ignore this filter and return all the routing information. It shouldn't happen. Instead, when unsupported filter type is given, the function should response with 400 error and notify user there exist unsupported filter types. 2. What is the solution to the problem Response with 400 error when unsupported filter type is specified for routing's get_all(self, **kwargs) function. 3. What the features need to be implemented to the Tricircle to realize the solution Modify the original resource routing's get_all(self, **kwargs) function to return 400 error when met unsupported filter type. Change-Id: I1bff9ebb7a6371a139d9ba547af417548100a2b4 Closes-Bug: #1690971
This commit is contained in:
parent
50067c3b4b
commit
1d20ef03b8
|
@ -202,8 +202,9 @@ class AsyncJobController(rest.RestController):
|
|||
is_valid_filter, filters = self._get_filters(kwargs)
|
||||
|
||||
if not is_valid_filter:
|
||||
msg = (_('Unsupported filter type: %(filter)s') %
|
||||
{'filter': [filter_name for filter_name in filters]})
|
||||
msg = (_('Unsupported filter type: %(filters)s') % {
|
||||
'filters': ', '.join([filter_name for filter_name in filters])
|
||||
})
|
||||
return utils.format_api_error(400, msg)
|
||||
|
||||
filters = [{'key': key,
|
||||
|
|
|
@ -91,16 +91,21 @@ class RoutingController(rest.RestController):
|
|||
"""Return a dictionary of query param filters from the request.
|
||||
|
||||
:param params: the URI params coming from the wsgi layer
|
||||
:return a dict of key/value filters
|
||||
:return (flag, filters), flag indicates whether the filters are valid,
|
||||
and the filters denote a list of key-value pairs.
|
||||
"""
|
||||
filters = {}
|
||||
for param in params:
|
||||
if param in SUPPORTED_FILTERS:
|
||||
unsupported_filters = {}
|
||||
for filter_name in params:
|
||||
if filter_name in SUPPORTED_FILTERS:
|
||||
# map filter name
|
||||
filter_name = param
|
||||
filters[filter_name] = params.get(param)
|
||||
filters[filter_name] = params.get(filter_name)
|
||||
else:
|
||||
unsupported_filters[filter_name] = params.get(filter_name)
|
||||
|
||||
return filters
|
||||
if unsupported_filters:
|
||||
return False, unsupported_filters
|
||||
return True, filters
|
||||
|
||||
@expose(generic=True, template='json')
|
||||
def get_all(self, **kwargs):
|
||||
|
@ -110,7 +115,14 @@ class RoutingController(rest.RestController):
|
|||
return utils.format_api_error(
|
||||
403, _('Unauthorized to show all resource routings'))
|
||||
|
||||
filters = self._get_filters(kwargs)
|
||||
is_valid_filter, filters = self._get_filters(kwargs)
|
||||
|
||||
if not is_valid_filter:
|
||||
msg = (_('Unsupported filter type: %(filters)s') % {
|
||||
'filters': ', '.join([filter_name for filter_name in filters])
|
||||
})
|
||||
return utils.format_api_error(400, msg)
|
||||
|
||||
filters = [{'key': key,
|
||||
'comparator': 'eq',
|
||||
'value': value} for key, value in six.iteritems(filters)]
|
||||
|
|
|
@ -253,14 +253,15 @@ class RoutingControllerTest(unittest.TestCase):
|
|||
routings = self.controller.get_all(**kw_filter2)
|
||||
self.assertEqual([], routings['routings'])
|
||||
|
||||
# apply an illegal filter and it won't take effect
|
||||
# failure case, use an unsupported filter type
|
||||
kw_filter3 = {'resource': 'port'}
|
||||
routings = self.controller.get_all(**kw_filter3)
|
||||
actual = [(routing['top_id'], routing['pod_id'])
|
||||
for routing in routings['routings']]
|
||||
expect = [('c7f641c9-8462-4007-84b2-3035d8cfb7a3', pod_id1),
|
||||
('b669a2da-ca95-47db-a2a9-ba9e546d82ee', pod_id2)]
|
||||
six.assertCountEqual(self, expect, actual)
|
||||
res = self.controller.get_all(**kw_filter3)
|
||||
self._validate_error_code(res, 400)
|
||||
|
||||
kw_filter4 = {'pod_id': pod_id1,
|
||||
'resource': 'port'}
|
||||
res = self.controller.get_all(**kw_filter4)
|
||||
self._validate_error_code(res, 400)
|
||||
|
||||
# failure case, only admin can show all resource routings
|
||||
self.context.is_admin = False
|
||||
|
|
Loading…
Reference in New Issue