From adc958f2534fdcc36dcdf974fd95067843e4c14c Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Mon, 6 Jun 2016 12:00:50 -0400 Subject: [PATCH] Pass context down to ViewBuilder method The versioned_method decorator helps add/remove fields in API responses. The method reads the request object but does not pass down the context to the called methods. The context would be helpful to make decisions regarding whether the fields are specific to an administrator context or owner/tenant context. Change-Id: Ic2e2df305362fa3f353fa2dfd75203afc38afb82 Closes-Bug: #1589516 --- manila/api/common.py | 3 +- manila/api/views/export_locations.py | 3 +- manila/api/views/share_instance.py | 8 ++-- manila/api/views/share_snapshots.py | 13 +++---- manila/api/views/shares.py | 16 ++++---- manila/api/views/types.py | 6 ++- manila/tests/api/fakes.py | 33 ++++++++++++++++ manila/tests/api/test_common.py | 58 ++++++++++++++++++++++++++++ 8 files changed, 118 insertions(+), 22 deletions(-) diff --git a/manila/api/common.py b/manila/api/common.py index 93988a6af7..5f748ae468 100644 --- a/manila/api/common.py +++ b/manila/api/common.py @@ -273,7 +273,8 @@ class ViewBuilder(object): for method_name in self._detail_version_modifiers: method = getattr(self, method_name) if request.api_version_request.matches_versioned_method(method): - method.func(self, resource_dict, resource) + request_context = request.environ['manila.context'] + method.func(self, request_context, resource_dict, resource) @classmethod def versioned_method(cls, min_ver, max_ver=None, experimental=False): diff --git a/manila/api/views/export_locations.py b/manila/api/views/export_locations.py index 11cbd20c59..8bc96de019 100644 --- a/manila/api/views/export_locations.py +++ b/manila/api/views/export_locations.py @@ -77,6 +77,7 @@ class ViewBuilder(common.ViewBuilder): detail=False) @common.ViewBuilder.versioned_method('2.14') - def add_preferred_path_attribute(self, view_dict, export_location): + def add_preferred_path_attribute(self, context, view_dict, + export_location): view_dict['preferred'] = strutils.bool_from_string( export_location['el_metadata'].get('preferred')) diff --git a/manila/api/views/share_instance.py b/manila/api/views/share_instance.py index d8e7905139..0259f24887 100644 --- a/manila/api/views/share_instance.py +++ b/manila/api/views/share_instance.py @@ -63,16 +63,18 @@ class ViewBuilder(common.ViewBuilder): return instances_dict @common.ViewBuilder.versioned_method("2.9") - def remove_export_locations(self, share_instance_dict, share_instance): + def remove_export_locations(self, context, share_instance_dict, + share_instance): share_instance_dict.pop('export_location') share_instance_dict.pop('export_locations') @common.ViewBuilder.versioned_method("2.10") - def add_access_rules_status_field(self, instance_dict, share_instance): + def add_access_rules_status_field(self, context, instance_dict, + share_instance): instance_dict['access_rules_status'] = ( share_instance.get('access_rules_status') ) @common.ViewBuilder.versioned_method("2.11") - def add_replication_fields(self, instance_dict, share_instance): + def add_replication_fields(self, context, instance_dict, share_instance): instance_dict['replica_state'] = share_instance.get('replica_state') diff --git a/manila/api/views/share_snapshots.py b/manila/api/views/share_snapshots.py index f2c3a3dea4..a71c2c967a 100644 --- a/manila/api/views/share_snapshots.py +++ b/manila/api/views/share_snapshots.py @@ -57,17 +57,16 @@ class ViewBuilder(common.ViewBuilder): 'links': self._get_links(request, snapshot['id']), } - # NOTE(xyang): Only retrieve provider_location for admin. - context = request.environ['manila.context'] - if context.is_admin: - self.update_versioned_resource_dict(request, snapshot_dict, - snapshot) + self.update_versioned_resource_dict(request, snapshot_dict, snapshot) return {'snapshot': snapshot_dict} @common.ViewBuilder.versioned_method("2.12") - def add_provider_location_field(self, snapshot_dict, snapshot): - snapshot_dict['provider_location'] = snapshot.get('provider_location') + def add_provider_location_field(self, context, snapshot_dict, snapshot): + # NOTE(xyang): Only retrieve provider_location for admin. + if context.is_admin: + snapshot_dict['provider_location'] = snapshot.get( + 'provider_location') def _list_view(self, func, request, snapshots): """Provide a view for a list of share snapshots.""" diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index ed67ddecd0..0e3e569cb4 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -104,22 +104,22 @@ class ViewBuilder(common.ViewBuilder): return result @common.ViewBuilder.versioned_method("2.2") - def add_snapshot_support_field(self, share_dict, share): + def add_snapshot_support_field(self, context, share_dict, share): share_dict['snapshot_support'] = share.get('snapshot_support') @common.ViewBuilder.versioned_method("2.4") - def add_consistency_group_fields(self, share_dict, share): + def add_consistency_group_fields(self, context, share_dict, share): share_dict['consistency_group_id'] = share.get( 'consistency_group_id') share_dict['source_cgsnapshot_member_id'] = share.get( 'source_cgsnapshot_member_id') @common.ViewBuilder.versioned_method("2.5") - def add_task_state_field(self, share_dict, share): + def add_task_state_field(self, context, share_dict, share): share_dict['task_state'] = share.get('task_state') @common.ViewBuilder.versioned_method("2.6") - def modify_share_type_field(self, share_dict, share): + def modify_share_type_field(self, context, share_dict, share): share_type = share.get('share_type_id') share_type_name = None @@ -132,21 +132,21 @@ class ViewBuilder(common.ViewBuilder): }) @common.ViewBuilder.versioned_method("2.9") - def remove_export_locations(self, share_dict, share): + def remove_export_locations(self, context, share_dict, share): share_dict.pop('export_location') share_dict.pop('export_locations') @common.ViewBuilder.versioned_method("2.10") - def add_access_rules_status_field(self, share_dict, share): + def add_access_rules_status_field(self, context, share_dict, share): share_dict['access_rules_status'] = share.get('access_rules_status') @common.ViewBuilder.versioned_method('2.11') - def add_replication_fields(self, share_dict, share): + def add_replication_fields(self, context, share_dict, share): share_dict['replication_type'] = share.get('replication_type') share_dict['has_replicas'] = share['has_replicas'] @common.ViewBuilder.versioned_method("2.16") - def add_user_id(self, share_dict, share): + def add_user_id(self, context, share_dict, share): share_dict['user_id'] = share.get('user_id') def _list_view(self, func, request, shares): diff --git a/manila/api/views/types.py b/manila/api/views/types.py index 085ff4ba62..39b3c5ea56 100644 --- a/manila/api/views/types.py +++ b/manila/api/views/types.py @@ -53,12 +53,14 @@ class ViewBuilder(common.ViewBuilder): return dict(volume_type=trimmed, share_type=trimmed) @common.ViewBuilder.versioned_method("2.7") - def add_is_public_attr_core_api_like(self, share_type_dict, share_type): + def add_is_public_attr_core_api_like(self, context, share_type_dict, + share_type): share_type_dict['share_type_access:is_public'] = share_type.get( 'is_public', True) @common.ViewBuilder.versioned_method("1.0", "2.6") - def add_is_public_attr_extension_like(self, share_type_dict, share_type): + def add_is_public_attr_extension_like(self, context, share_type_dict, + share_type): share_type_dict['os-share-type-access:is_public'] = share_type.get( 'is_public', True) diff --git a/manila/tests/api/fakes.py b/manila/tests/api/fakes.py index 8f5e23a1f0..c860e3eef5 100644 --- a/manila/tests/api/fakes.py +++ b/manila/tests/api/fakes.py @@ -21,6 +21,7 @@ import webob import webob.dec import webob.request +from manila.api import common as api_common from manila.api.middleware import auth from manila.api.middleware import fault from manila.api.openstack import api_version_request as api_version @@ -282,3 +283,35 @@ def mock_fake_admin_check(context, resource_name, action, *args, **kwargs): return else: raise exception.PolicyNotAuthorized(action=action) + + +class FakeResourceViewBuilder(api_common.ViewBuilder): + + _collection_name = 'fake_resource' + _detail_version_modifiers = [ + "add_field_xyzzy", + "add_field_spoon_for_admins", + "remove_field_foo", + ] + + def view(self, req, resource): + + keys = ('id', 'foo', 'fred', 'alice') + resource_dict = {key: resource.get(key) for key in keys} + + self.update_versioned_resource_dict(req, resource_dict, resource) + + return resource_dict + + @api_common.ViewBuilder.versioned_method("1.41") + def add_field_xyzzy(self, context, resource_dict, resource): + resource_dict['xyzzy'] = resource.get('xyzzy') + + @api_common.ViewBuilder.versioned_method("1.6") + def add_field_spoon_for_admins(self, context, resource_dict, resource): + if context.is_admin: + resource_dict['spoon'] = resource.get('spoon') + + @api_common.ViewBuilder.versioned_method("3.14") + def remove_field_foo(self, context, resource_dict, resource): + resource_dict.pop('foo', None) diff --git a/manila/tests/api/test_common.py b/manila/tests/api/test_common.py index 8581008abb..4da0856d9c 100644 --- a/manila/tests/api/test_common.py +++ b/manila/tests/api/test_common.py @@ -17,11 +17,14 @@ Test suites for 'common' code used throughout the OpenStack HTTP API. """ +import ddt import webob import webob.exc from manila.api import common from manila import test +from manila.tests.api import fakes +from manila.tests.db import fakes as db_fakes NS = "{http://docs.openstack.org/compute/api/v1.1}" @@ -240,3 +243,58 @@ class MiscFunctionsTest(test.TestCase): self.assertRaises(ValueError, common.remove_version_from_href, fixture) + + +@ddt.ddt +class ViewBuilderTest(test.TestCase): + + def setUp(self): + super(ViewBuilderTest, self).setUp() + self.expected_resource_dict = { + 'id': 'fake_resource_id', + 'foo': 'quz', + 'fred': 'bob', + 'alice': 'waldo', + 'spoon': 'spam', + 'xyzzy': 'qwerty', + } + self.fake_resource = db_fakes.FakeModel(self.expected_resource_dict) + self.view_builder = fakes.FakeResourceViewBuilder() + + @ddt.data('1.0', '1.40') + def test_versioned_method_no_updates(self, version): + req = fakes.HTTPRequest.blank('/my_resource', version=version) + + actual_resource = self.view_builder.view(req, self.fake_resource) + + self.assertEqual(set({'id', 'foo', 'fred', 'alice'}), + set(actual_resource.keys())) + + @ddt.data(True, False) + def test_versioned_method_v1_6(self, is_admin): + req = fakes.HTTPRequest.blank('/my_resource', version='1.6', + use_admin_context=is_admin) + expected_keys = set({'id', 'foo', 'fred', 'alice'}) + if is_admin: + expected_keys.add('spoon') + + actual_resource = self.view_builder.view(req, self.fake_resource) + + self.assertEqual(expected_keys, set(actual_resource.keys())) + + @ddt.unpack + @ddt.data({'is_admin': True, 'version': '3.14'}, + {'is_admin': False, 'version': '3.14'}, + {'is_admin': False, 'version': '6.2'}, + {'is_admin': True, 'version': '6.2'}) + def test_versioned_method_all_match(self, is_admin, version): + req = fakes.HTTPRequest.blank( + '/my_resource', version=version, use_admin_context=is_admin) + + expected_keys = set({'id', 'fred', 'xyzzy', 'alice'}) + if is_admin: + expected_keys.add('spoon') + + actual_resource = self.view_builder.view(req, self.fake_resource) + + self.assertEqual(expected_keys, set(actual_resource.keys()))