openstack
/
neutron
Code Issues Proposed changes

Remove REVERSED_PLURALS and get_resource_info() 

By changing pecan to store plugins by collection (plural) instead of
resource (singular) we don't need REVERSED_PLURALS and
get_resource_info().

These are not used outside of neutron core.

Consequently, this exposed a bug with some controllers not honoring an
extension's intention of disallowing pagination and sorting.  This is
also fixed in this patch to get successful test runs.

Co-Authored-By: Brandon Logan <brandon.logan@rackspace.com>

Change-Id: I9d529d0a2ad369e2be0a8df3c6f06a6532e8b13d
This commit is contained in:
Henry Gessau 2016-11-16 11:33:37 -05:00
parent b9d0a5b885
commit 4151486bd4
9 changed files with 33 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
neutron/api/v2/attributes.py
View File

@ -297,9 +297,6 @@ PLURALS = {NETWORKS: NETWORK,
'allocation_pools': 'allocation_pool',
'fixed_ips': 'fixed_ip',
'extensions': 'extension'}
# Store singular/plural mappings. This dictionary is populated by
# get_resource_info
REVERSED_PLURALS = {}
def get_collection_info(collection):
@ -310,20 +307,6 @@ def get_collection_info(collection):
return RESOURCE_ATTRIBUTE_MAP.get(collection)
def get_resource_info(resource):
"""Helper function to retrieve attribute info
:param resource: resource name
"""
plural_name = REVERSED_PLURALS.get(resource)
if not plural_name:
for (plural, singular) in PLURALS.items():
if singular == resource:
plural_name = plural
REVERSED_PLURALS[resource] = plural_name
return RESOURCE_ATTRIBUTE_MAP.get(plural_name)
def fill_default_value(attr_info, res_dict,
exc_cls=ValueError,
check_allow_post=True):

8
neutron/api/v2/base.py
View File

@ -75,6 +75,14 @@ class Controller(object):
def member_actions(self):
return self._member_actions
@property
def allow_pagination(self):
return self._allow_pagination
@property
def allow_sorting(self):
return self._allow_sorting
def _init_policy_attrs(self):
"""Create the list of attributes required by policy.

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

@ -122,7 +122,8 @@ class CollectionsController(utils.NeutronPecanController):
# NOTE(tonytan4ever): item needs to share the same
# parent as collection
parent_resource=self.parent,
member_actions=self._member_actions), remainder)
member_actions=self._member_actions,
plugin=self.plugin), remainder)
@utils.expose(generic=True)
def index(self, *args, **kwargs):

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

@ -150,7 +150,7 @@ class NeutronPecanController(object):
def plugin(self):
if not self._plugin:
self._plugin = manager.NeutronManager.get_plugin_for_resource(
self.resource)
self.collection)
return self._plugin
@property

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

@ -36,10 +36,11 @@ def _custom_getter(resource, resource_id):
return quota.get_tenant_quotas(resource_id)[quotasv2.RESOURCE_NAME]
def fetch_resource(state, neutron_context, controller, resource, resource_id,
def fetch_resource(method, neutron_context, controller,
collection, resource, resource_id,
parent_id=None):
field_list = []
if state.request.method == 'PUT':
if method == 'PUT':
attrs = controller.resource_info
if not attrs:
# this isn't a request for a normal resource. it could be
@ -50,7 +51,7 @@ def fetch_resource(state, neutron_context, controller, resource, resource_id,
field_list = [name for (name, value) in attrs.items()
if (value.get('required_by_policy') or
value.get('primary_key') or 'default' not in value)]
plugin = manager.NeutronManager.get_plugin_for_resource(resource)
plugin = manager.NeutronManager.get_plugin_for_resource(collection)
if plugin:
if utils.is_member_action(controller):
getter = controller.parent_controller.plugin_shower
@ -109,8 +110,9 @@ class PolicyHook(hooks.PecanHook):
item = {}
resource_id = state.request.context.get('resource_id')
parent_id = state.request.context.get('parent_id')
resource_obj = fetch_resource(state, neutron_context, controller,
resource, resource_id,
method = state.request.method
resource_obj = fetch_resource(method, neutron_context, controller,
collection, resource, resource_id,
parent_id=parent_id)
if resource_obj:
original_resources.append(resource_obj)
@ -169,7 +171,7 @@ class PolicyHook(hooks.PecanHook):
# in the single case, we enforce which raises on violation
# in the plural case, we just check so violating items are hidden
policy_method = policy.enforce if is_single else policy.check
plugin = manager.NeutronManager.get_plugin_for_resource(resource)
plugin = manager.NeutronManager.get_plugin_for_resource(collection)
try:
resp = [self._get_filtered_item(state.request, controller,
resource, collection, item)

3
neutron/pecan_wsgi/hooks/quota_enforcement.py
View File

@ -31,11 +31,12 @@ class QuotaEnforcementHook(hooks.PecanHook):
priority = 130
def before(self, state):
collection = state.request.context.get('collection')
resource = state.request.context.get('resource')
items = state.request.context.get('resources')
if state.request.method != 'POST' or not resource or not items:
return
plugin = manager.NeutronManager.get_plugin_for_resource(resource)
plugin = manager.NeutronManager.get_plugin_for_resource(collection)
# Store requested resource amounts grouping them by tenant
deltas = collections.Counter(map(lambda x: x['tenant_id'], items))
# Perform quota enforcement

15
neutron/pecan_wsgi/startup.py
View File

@ -44,15 +44,14 @@ def initialize_all():
plugin=plugin)
manager.NeutronManager.set_controller_for_resource(
collection, new_controller)
manager.NeutronManager.set_plugin_for_resource(resource, plugin)
manager.NeutronManager.set_plugin_for_resource(collection, plugin)
pecanized_resources = ext_mgr.get_pecan_resources()
for pec_res in pecanized_resources:
resource = attributes.PLURALS[pec_res.collection]
manager.NeutronManager.set_controller_for_resource(
pec_res.collection, pec_res.controller)
manager.NeutronManager.set_plugin_for_resource(
resource, pec_res.plugin)
pec_res.collection, pec_res.plugin)
# Now build Pecan Controllers and routes for all extensions
resources = ext_mgr.get_resources()
@ -84,10 +83,16 @@ def initialize_all():
plugin = legacy_controller.plugin
attr_info = legacy_controller.attr_info
member_actions = legacy_controller.member_actions
pagination = legacy_controller.allow_pagination
sorting = legacy_controller.allow_sorting
new_controller = res_ctrl.CollectionsController(
collection, resource, resource_info=attr_info,
parent_resource=parent_resource, member_actions=member_actions)
manager.NeutronManager.set_plugin_for_resource(resource, plugin)
parent_resource=parent_resource, member_actions=member_actions,
plugin=plugin, allow_pagination=pagination,
allow_sorting=sorting)
# new_controller.collection has replaced hyphens with underscores
manager.NeutronManager.set_plugin_for_resource(
new_controller.collection, plugin)
if path_prefix:
manager.NeutronManager.add_resource_for_path_prefix(
collection, path_prefix)

3
neutron/tests/functional/pecan_wsgi/test_hooks.py
View File

@ -114,7 +114,8 @@ class TestPolicyEnforcementHook(test_functional.PecanFunctionalTest):
self.mock_plugin = mock.Mock()
attributes.RESOURCE_ATTRIBUTE_MAP.update(self.FAKE_RESOURCE)
attributes.PLURALS['mehs'] = 'meh'
manager.NeutronManager.set_plugin_for_resource('meh', self.mock_plugin)
manager.NeutronManager.set_plugin_for_resource('mehs',
self.mock_plugin)
fake_controller = resource.CollectionsController('mehs', 'meh')
manager.NeutronManager.set_controller_for_resource(
'mehs', fake_controller)

18
neutron/tests/unit/api/v2/test_attributes.py
View File

@ -13,7 +13,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import mock
from neutron_lib.api import converters
from neutron_lib import constants
from neutron_lib import exceptions as n_exc
@ -156,20 +155,3 @@ class TestHelpers(base.DietTestCase):
def test_get_collection_info_missing(self):
self.assertFalse(attributes.get_collection_info('meh'))
def test_get_resource_info(self):
attributes.REVERSED_PLURALS.pop('port', None)
attrs = attributes.get_resource_info('port')
self._verify_port_attributes(attrs)
# verify side effect
self.assertIn('port', attributes.REVERSED_PLURALS)
def test_get_resource_info_missing(self):
self.assertFalse(attributes.get_resource_info('meh'))
def test_get_resource_info_cached(self):
with mock.patch('neutron.api.v2.attributes.PLURALS') as mock_plurals:
attributes.REVERSED_PLURALS['port'] = 'ports'
attrs = attributes.get_resource_info('port')
self._verify_port_attributes(attrs)
self.assertEqual(0, mock_plurals.items.call_count)