openstack
/
neutron
Code Issues Proposed changes

Pecan: tell the plugin about field selection 

Neutron plugin are able to do field selection on responses (for
instance only returning id & name for a resource). However,
Pecan does not leverage this capability and always tells the
plugin to fetch all fields, doing field selection while
processing the response.

This patch ensures that pecan send the field list down to the
plugin.

As a part of this patch TestRequestProcessing has been update
to inherit from TestRootController rather than
TestResourceController. Inheriting from the latter was causing
tests to be executed twice for no reason, beyong using
TestResourceController's 'port' attribute, which was however
unnecessary as this change proves.

Closes-Bug: #1570259

Change-Id: Iac930cd3bb14dfdda78e6a94d2c8bef2b5c4b9a5
This commit is contained in:
salvatore 2016-04-14 11:34:04 +02:00 committed by Brandon Logan
parent d7268dd5ef
commit 80426cf620
4 changed files with 76 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
neutron/pecan_wsgi/controllers/resource.py
View File

@ -38,7 +38,9 @@ class ItemController(utils.NeutronPecanController):
def get(self, *args, **kwargs):
getter = getattr(self.plugin, 'get_%s' % self.resource)
neutron_context = request.context['neutron_context']
return {self.resource: getter(neutron_context, self.item)}
fields = self._build_field_list(kwargs.pop('fields', []))
return {self.resource: getter(neutron_context, self.item,
fields=fields)}
@utils.when(index, method='HEAD')
@utils.when(index, method='POST')
@ -95,9 +97,7 @@ class CollectionsController(utils.NeutronPecanController):
def get(self, *args, **kwargs):
# list request
# TODO(kevinbenton): use user-provided fields in call to plugin
# after making sure policy enforced fields remain
kwargs.pop('fields', None)
fields = self._build_field_list(kwargs.pop('fields', []))
_listify = lambda x: x if isinstance(x, list) else [x]
filters = api_common.get_filters_from_dict(
{k: _listify(v) for k, v in kwargs.items()},
@ -106,7 +106,8 @@ class CollectionsController(utils.NeutronPecanController):
'limit', 'marker', 'page_reverse'])
lister = getattr(self.plugin, 'get_%s' % self.collection)
neutron_context = request.context['neutron_context']
return {self.collection: lister(neutron_context, filters=filters)}
return {self.collection: lister(neutron_context,
fields=fields, filters=filters)}
@utils.when(index, method='HEAD')
@utils.when(index, method='PATCH')

11
neutron/pecan_wsgi/controllers/utils.py
View File

@ -94,6 +94,17 @@ class NeutronPecanController(object):
self._resource_info = api_attributes.get_collection_info(
self.collection)
self._plugin = plugin
# Controllers for some resources that are not mapped to anything in
# RESOURCE_ATTRIBUTE_MAP will not have anything in _resource_info
if self._resource_info:
self._mandatory_fields = set([field for (field, data) in
self._resource_info.items() if
data.get('required_by_policy')])
else:
self._mandatory_fields = set()
def _build_field_list(self, request_fields):
return set(request_fields) | self._mandatory_fields
@property
def plugin(self):

6
neutron/pecan_wsgi/hooks/policy_enforcement.py
View File

@ -178,9 +178,9 @@ class PolicyHook(hooks.PecanHook):
return self._filter_attributes(request, data, to_exclude)
def _filter_attributes(self, request, data, fields_to_strip):
# TODO(kevinbenton): this works but we didn't allow the plugin to
# only fetch the fields we are interested in. consider moving this
# to the call
# This routine will remove the fields that were requested to the
# plugin for policy evaluation but were not specified in the
# API request
user_fields = request.params.getall('fields')
return dict(item for item in data.items()
if (item[0] not in fields_to_strip and

89
neutron/tests/functional/pecan_wsgi/test_controllers.py
View File

@ -258,6 +258,36 @@ class TestResourceController(TestRootController):
response = self.app.get('/v2.0/ports.json')
self.assertEqual(response.status_int, 200)
def _check_item(self, expected, item):
for attribute in expected:
self.assertIn(attribute, item)
self.assertEqual(len(expected), len(item))
def test_get_collection_with_fields_selector(self):
list_resp = self.app.get('/v2.0/ports.json?fields=id&fields=name',
headers={'X-Project-Id': 'tenid'})
self.assertEqual(200, list_resp.status_int)
for item in jsonutils.loads(list_resp.body).get('ports', []):
self.assertIn('id', item)
self.assertIn('name', item)
self.assertEqual(2, len(item))
def test_get_item_with_fields_selector(self):
item_resp = self.app.get(
'/v2.0/ports/%s.json?fields=id&fields=name' % self.port['id'],
headers={'X-Project-Id': 'tenid'})
self.assertEqual(200, item_resp.status_int)
self._check_item(['id', 'name'],
jsonutils.loads(item_resp.body)['port'])
# Explicitly require an attribute which is also 'required_by_policy'.
# The attribute should not be stripped while generating the response
item_resp = self.app.get(
'/v2.0/ports/%s.json?fields=id&fields=tenant_id' % self.port['id'],
headers={'X-Project-Id': 'tenid'})
self.assertEqual(200, item_resp.status_int)
self._check_item(['id', 'tenant_id'],
jsonutils.loads(item_resp.body)['port'])
def test_post(self):
response = self.app.post_json(
'/v2.0/ports.json',
@ -315,7 +345,7 @@ class TestResourceController(TestRootController):
self._test_method_returns_code('delete', 405)
class TestRequestProcessing(TestResourceController):
class TestRequestProcessing(TestRootController):
def setUp(self):
super(TestRequestProcessing, self).setUp()
@ -326,6 +356,7 @@ class TestRequestProcessing(TestResourceController):
def capture_request_details(*args, **kwargs):
self.captured_context = request.context
self.request_params = kwargs
mock.patch('neutron.pecan_wsgi.controllers.resource.'
'CollectionsController.get',
@ -359,55 +390,47 @@ class TestRequestProcessing(TestResourceController):
def test_resource_processing_post(self):
self.app.post_json(
'/v2.0/ports.json',
params={'port': {'network_id': self.port['network_id'],
'name': 'the_port',
'admin_state_up': True}},
'/v2.0/networks.json',
params={'network': {'name': 'the_net',
'admin_state_up': True}},
headers={'X-Project-Id': 'tenid'})
self.assertEqual('port', self.captured_context['resource'])
self.assertEqual('ports', self.captured_context['collection'])
self.assertEqual('network', self.captured_context['resource'])
self.assertEqual('networks', self.captured_context['collection'])
resources = self.captured_context['resources']
self.assertEqual(1, len(resources))
self.assertEqual(self.port['network_id'],
resources[0]['network_id'])
self.assertEqual('the_port', resources[0]['name'])
self.assertEqual('the_net', resources[0]['name'])
self.assertTrue(resources[0]['admin_state_up'])
def test_resource_processing_post_bulk(self):
self.app.post_json(
'/v2.0/ports.json',
params={'ports': [{'network_id': self.port['network_id'],
'name': 'the_port_1',
'admin_state_up': True},
{'network_id': self.port['network_id'],
'name': 'the_port_2',
'admin_state_up': True}]},
'/v2.0/networks.json',
params={'networks': [{'name': 'the_net_1',
'admin_state_up': True},
{'name': 'the_net_2',
'admin_state_up': False}]},
headers={'X-Project-Id': 'tenid'})
resources = self.captured_context['resources']
self.assertEqual(2, len(resources))
self.assertEqual(self.port['network_id'],
resources[0]['network_id'])
self.assertEqual('the_port_1', resources[0]['name'])
self.assertEqual(self.port['network_id'],
resources[1]['network_id'])
self.assertEqual('the_port_2', resources[1]['name'])
self.assertTrue(resources[0]['admin_state_up'])
self.assertEqual('the_net_1', resources[0]['name'])
self.assertFalse(resources[1]['admin_state_up'])
self.assertEqual('the_net_2', resources[1]['name'])
def test_resource_processing_post_unknown_attribute_returns_400(self):
response = self.app.post_json(
'/v2.0/ports.json',
params={'port': {'network_id': self.port['network_id'],
'name': 'the_port',
'alien': 'E.T.',
'admin_state_up': True}},
'/v2.0/networks.json',
params={'network': {'name': 'the_net',
'alien': 'E.T.',
'admin_state_up': True}},
headers={'X-Project-Id': 'tenid'},
expect_errors=True)
self.assertEqual(400, response.status_int)
def test_resource_processing_post_validation_errori_returns_400(self):
def test_resource_processing_post_validation_error_returns_400(self):
response = self.app.post_json(
'/v2.0/ports.json',
params={'port': {'network_id': self.port['network_id'],
'name': 'the_port',
'admin_state_up': 'invalid_value'}},
'/v2.0/networks.json',
params={'network': {'name': 'the_net',
'admin_state_up': 'invalid_value'}},
headers={'X-Project-Id': 'tenid'},
expect_errors=True)
self.assertEqual(400, response.status_int)