diff --git a/neutron/api/api_common.py b/neutron/api/api_common.py index f9e7e9a5998..50d02c52681 100644 --- a/neutron/api/api_common.py +++ b/neutron/api/api_common.py @@ -186,6 +186,18 @@ def get_pagination_links(request, items, limit, return links +def is_native_pagination_supported(plugin): + native_pagination_attr_name = ("_%s__native_pagination_support" + % plugin.__class__.__name__) + return getattr(plugin, native_pagination_attr_name, False) + + +def is_native_sorting_supported(plugin): + native_sorting_attr_name = ("_%s__native_sorting_support" + % plugin.__class__.__name__) + return getattr(plugin, native_sorting_attr_name, False) + + class PaginationHelper(object): def __init__(self, request, primary_key='id'): diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index a86ed042537..3de986c1de2 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -128,14 +128,10 @@ class Controller(object): return getattr(self._plugin, native_bulk_attr_name, False) def _is_native_pagination_supported(self): - native_pagination_attr_name = ("_%s__native_pagination_support" - % self._plugin.__class__.__name__) - return getattr(self._plugin, native_pagination_attr_name, False) + return api_common.is_native_pagination_supported(self._plugin) def _is_native_sorting_supported(self): - native_sorting_attr_name = ("_%s__native_sorting_support" - % self._plugin.__class__.__name__) - return getattr(self._plugin, native_sorting_attr_name, False) + return api_common.is_native_sorting_supported(self._plugin) def _exclude_attributes_by_policy(self, context, data): """Identifies attributes to exclude according to authZ policies. diff --git a/neutron/pecan_wsgi/app.py b/neutron/pecan_wsgi/app.py index dcde26bdf44..51d65ecbcbe 100644 --- a/neutron/pecan_wsgi/app.py +++ b/neutron/pecan_wsgi/app.py @@ -50,8 +50,8 @@ def setup_app(*args, **kwargs): hooks.OwnershipValidationHook(), # priority 125 hooks.QuotaEnforcementHook(), # priority 130 hooks.NotifierHook(), # priority 135 + hooks.QueryParametersHook(), # priority 139 hooks.PolicyHook(), # priority 140 - hooks.QueryParametersHook(), # priority 145 ] app = pecan.make_app( diff --git a/neutron/pecan_wsgi/controllers/resource.py b/neutron/pecan_wsgi/controllers/resource.py index d5304d69891..1c17c3cd5ef 100644 --- a/neutron/pecan_wsgi/controllers/resource.py +++ b/neutron/pecan_wsgi/controllers/resource.py @@ -98,13 +98,11 @@ class CollectionsController(utils.NeutronPecanController): return self.get(*args, **kwargs) def get(self, *args, **kwargs): - # NOTE(blogan): these are set in the FieldsAndFiltersHoook - fields = request.context['query_params'].get('fields') - filters = request.context['query_params'].get('filters') + # NOTE(blogan): query_params is set in the QueryParametersHook + query_params = request.context['query_params'] lister = getattr(self.plugin, 'get_%s' % self.collection) neutron_context = request.context['neutron_context'] - return {self.collection: lister(neutron_context, - fields=fields, filters=filters)} + return {self.collection: lister(neutron_context, **query_params)} @utils.when(index, method='HEAD') @utils.when(index, method='PATCH') diff --git a/neutron/pecan_wsgi/controllers/utils.py b/neutron/pecan_wsgi/controllers/utils.py index 1ac042f7a83..84a149872b7 100644 --- a/neutron/pecan_wsgi/controllers/utils.py +++ b/neutron/pecan_wsgi/controllers/utils.py @@ -17,9 +17,12 @@ import copy import functools from neutron_lib import constants +from oslo_config import cfg import pecan from pecan import request +import six +from neutron.api import api_common from neutron.api.v2 import attributes as api_attributes from neutron.db import api as db_api from neutron import manager @@ -87,7 +90,8 @@ def when(index, *args, **kwargs): class NeutronPecanController(object): - def __init__(self, collection, resource, plugin=None, resource_info=None): + def __init__(self, collection, resource, plugin=None, resource_info=None, + allow_pagination=None, allow_sorting=None): # Ensure dashes are always replaced with underscores self.collection = collection and collection.replace('-', '_') self.resource = resource and resource.replace('-', '_') @@ -101,11 +105,26 @@ class NeutronPecanController(object): data.get('required_by_policy')]) else: self._mandatory_fields = set() + self.allow_pagination = allow_pagination + if self.allow_pagination is None: + self.allow_pagination = cfg.CONF.allow_pagination + self.allow_sorting = allow_sorting + if self.allow_sorting is None: + self.allow_sorting = cfg.CONF.allow_sorting + self.native_pagination = api_common.is_native_pagination_supported( + self.plugin) + self.native_sorting = api_common.is_native_sorting_supported( + self.plugin) + self.primary_key = self._get_primary_key() def build_field_list(self, request_fields): + added_fields = [] + combined_fields = [] if request_fields: - return set(request_fields) | self._mandatory_fields - return [] + req_fields_set = set(request_fields) + added_fields = self._mandatory_fields - req_fields_set + combined_fields = req_fields_set | self._mandatory_fields + return list(combined_fields), list(added_fields) @property def plugin(self): @@ -121,6 +140,14 @@ class NeutronPecanController(object): self.collection) return self._resource_info + def _get_primary_key(self, default_primary_key='id'): + if not self.resource_info: + return default_primary_key + for key, value in six.iteritems(self.resource_info): + if value.get('primary_key', False): + return key + return default_primary_key + class ShimRequest(object): diff --git a/neutron/pecan_wsgi/hooks/query_parameters.py b/neutron/pecan_wsgi/hooks/query_parameters.py index 8815926721b..82c9316cbfd 100644 --- a/neutron/pecan_wsgi/hooks/query_parameters.py +++ b/neutron/pecan_wsgi/hooks/query_parameters.py @@ -14,6 +14,44 @@ from pecan import hooks from neutron.api import api_common from neutron import manager +from neutron.pecan_wsgi.hooks import policy_enforcement + + +# TODO(blogan): ideally it'd be nice to get the pagination and sorting +# helpers from the controller but since the controllers are +# instantiated at startup and not on request, it would cause race +# conditions because we need a new instantiation of a pagination +# and sorting helper per request/response flow. As a result, we're forced to +# pass them through the request context. + +def _get_pagination_helper(request, controller): + if 'pagination_helper' in request.context: + return request.context['pagination_helper'] + if not controller.allow_pagination: + helper = api_common.NoPaginationHelper(request, controller.primary_key) + elif controller.native_pagination: + helper = api_common.PaginationNativeHelper(request, + controller.primary_key) + else: + helper = api_common.PaginationEmulatedHelper(request, + controller.primary_key) + request.context['pagination_helper'] = helper + return helper + + +def _get_sorting_helper(request, controller): + if 'sorting_helper' in request.context: + return request.context['sorting_helper'] + if not controller.allow_sorting: + helper = api_common.NoSortingHelper(request, controller.resource_info) + elif controller.native_sorting: + helper = api_common.SortingNativeHelper(request, + controller.resource_info) + else: + helper = api_common.SortingEmulatedHelper(request, + controller.resource_info) + request.context['sorting_helper'] = helper + return helper def _listify(thing): @@ -26,8 +64,10 @@ def _set_fields(state, controller): # if only one fields query parameter is passed, pecan will not put # that parameter in a list, so we need to convert it into a list fields = _listify(fields) - combined_fields = controller.build_field_list(fields) - return combined_fields + combined_fields, added_fields = controller.build_field_list(fields) + state.request.context['query_params']['fields'] = combined_fields + state.request.context['added_fields'] = added_fields + return combined_fields, added_fields def _set_filters(state, controller): @@ -42,7 +82,9 @@ def _set_filters(state, controller): class QueryParametersHook(hooks.PecanHook): - priority = 145 + # NOTE(blogan): needs to be run after the priority hook. after methods + # are run in reverse priority order. + priority = policy_enforcement.PolicyHook.priority - 1 def before(self, state): state.request.context['query_params'] = {} @@ -53,7 +95,41 @@ class QueryParametersHook(hooks.PecanHook): return controller = manager.NeutronManager.get_controller_for_resource( collection) - combined_fields = _set_fields(state, controller) + combined_fields, added_fields = _set_fields(state, controller) filters = _set_filters(state, controller) query_params = {'fields': combined_fields, 'filters': filters} + pagination_helper = _get_pagination_helper(state.request, controller) + sorting_helper = _get_sorting_helper(state.request, controller) + sorting_helper.update_args(query_params) + sorting_helper.update_fields(query_params.get('fields', []), + added_fields) + pagination_helper.update_args(query_params) + pagination_helper.update_fields(query_params.get('fields', []), + added_fields) state.request.context['query_params'] = query_params + + def after(self, state): + resource = state.request.context.get('resource') + collection = state.request.context.get('collection') + # NOTE(blogan): don't paginate extension list or non-GET requests + if (not resource or resource == 'extension' or + state.request.method != 'GET'): + return + try: + data = state.response.json + except ValueError: + return + # Do not attempt to paginate if the body is not a list of entities + if not data or resource in data or collection not in data: + return + controller = manager.NeutronManager.get_controller_for_resource( + collection) + sorting_helper = _get_sorting_helper(state.request, controller) + pagination_helper = _get_pagination_helper(state.request, controller) + obj_list = sorting_helper.sort(data[collection]) + obj_list = pagination_helper.paginate(obj_list) + resp_body = {collection: obj_list} + pagination_links = pagination_helper.get_links(obj_list) + if pagination_links: + resp_body['_'.join([collection, 'links'])] = pagination_links + state.response.json = resp_body diff --git a/neutron/pecan_wsgi/startup.py b/neutron/pecan_wsgi/startup.py index 48bbcc39d14..33b3b52b565 100644 --- a/neutron/pecan_wsgi/startup.py +++ b/neutron/pecan_wsgi/startup.py @@ -38,7 +38,8 @@ def initialize_all(): for resource, collection in router.RESOURCES.items(): resource_registry.register_resource_by_name(resource) plugin = manager.NeutronManager.get_plugin() - new_controller = res_ctrl.CollectionsController(collection, resource) + new_controller = res_ctrl.CollectionsController(collection, resource, + plugin=plugin) manager.NeutronManager.set_controller_for_resource( collection, new_controller) manager.NeutronManager.set_plugin_for_resource(resource, plugin) diff --git a/neutron/tests/functional/pecan_wsgi/test_controllers.py b/neutron/tests/functional/pecan_wsgi/test_controllers.py index 0ca905d86b5..b45c863ddf4 100644 --- a/neutron/tests/functional/pecan_wsgi/test_controllers.py +++ b/neutron/tests/functional/pecan_wsgi/test_controllers.py @@ -365,6 +365,122 @@ class TestResourceController(TestRootController): self._test_method_returns_code('delete', 405) +class TestPaginationAndSorting(test_functional.PecanFunctionalTest): + + RESOURCE_COUNT = 6 + + def setUp(self): + cfg.CONF.set_override('allow_pagination', True) + cfg.CONF.set_override('allow_sorting', True) + super(TestPaginationAndSorting, self).setUp() + self.plugin = manager.NeutronManager.get_plugin() + self.ctx = context.get_admin_context() + self._create_networks(self.RESOURCE_COUNT) + self.networks = self._get_collection()['networks'] + + def _create_networks(self, count=1): + network_ids = [] + for index in range(count): + network = {'name': 'pecannet-%d' % index, 'tenant_id': 'tenid', + 'shared': False, 'admin_state_up': True, + 'status': 'ACTIVE'} + network_id = self.plugin.create_network( + self.ctx, {'network': network})['id'] + network_ids.append(network_id) + return network_ids + + def _get_collection(self, collection=None, limit=None, marker=None, + fields=None, page_reverse=False, sort_key=None, + sort_dir=None): + collection = collection or 'networks' + fields = fields or [] + query_params = [] + if limit: + query_params.append('limit=%d' % limit) + if marker: + query_params.append('marker=%s' % marker) + if page_reverse: + query_params.append('page_reverse=True') + if sort_key: + query_params.append('sort_key=%s' % sort_key) + if sort_dir: + query_params.append('sort_dir=%s' % sort_dir) + query_params.extend(['%s%s' % ('fields=', field) for field in fields]) + url = '/v2.0/%s.json' % collection + if query_params: + url = '%s?%s' % (url, '&'.join(query_params)) + list_resp = self.app.get(url, headers={'X-Project-Id': 'tenid'}) + self.assertEqual(200, list_resp.status_int) + return list_resp.json + + def _test_get_collection_with_pagination(self, expected_list, + collection=None, + limit=None, marker=None, + fields=None, page_reverse=False, + sort_key=None, sort_dir=None): + expected_list = expected_list or [] + collection = collection or 'networks' + list_resp = self._get_collection(collection=collection, limit=limit, + marker=marker, fields=fields, + page_reverse=page_reverse, + sort_key=sort_key, sort_dir=sort_dir) + if limit and marker: + links_key = '%s_links' % collection + self.assertIn(links_key, list_resp) + list_resp_ids = [item['id'] for item in list_resp[collection]] + self.assertEqual(expected_list, list_resp_ids) + if fields: + for item in list_resp[collection]: + for field in fields: + self.assertIn(field, item) + + def test_get_collection_with_pagination_limit(self): + self._test_get_collection_with_pagination([self.networks[0]['id']], + limit=1) + + def test_get_collection_with_pagination_limit_over_count(self): + expected_ids = [network['id'] for network in self.networks] + self._test_get_collection_with_pagination( + expected_ids, limit=self.RESOURCE_COUNT + 1) + + def test_get_collection_with_pagination_marker(self): + marker = self.networks[2]['id'] + expected_ids = [network['id'] for network in self.networks[3:]] + self._test_get_collection_with_pagination(expected_ids, limit=3, + marker=marker) + + def test_get_collection_with_pagination_marker_without_limit(self): + marker = self.networks[2]['id'] + expected_ids = [network['id'] for network in self.networks] + self._test_get_collection_with_pagination(expected_ids, marker=marker) + + def test_get_collection_with_pagination_and_fields(self): + expected_ids = [network['id'] for network in self.networks[:2]] + self._test_get_collection_with_pagination( + expected_ids, limit=2, fields=['id', 'name']) + + def test_get_collection_with_pagination_page_reverse(self): + marker = self.networks[2]['id'] + expected_ids = [network['id'] for network in self.networks[:2]] + self._test_get_collection_with_pagination(expected_ids, limit=3, + marker=marker, + page_reverse=True) + + def test_get_collection_with_sorting_desc(self): + nets = sorted(self.networks, key=lambda net: net['name'], reverse=True) + expected_ids = [network['id'] for network in nets] + self._test_get_collection_with_pagination(expected_ids, + sort_key='name', + sort_dir='desc') + + def test_get_collection_with_sorting_asc(self): + nets = sorted(self.networks, key=lambda net: net['name']) + expected_ids = [network['id'] for network in nets] + self._test_get_collection_with_pagination(expected_ids, + sort_key='name', + sort_dir='asc') + + class TestRequestProcessing(TestRootController): def setUp(self):