From 2e6969c3fe59184e5ae8bb9d327201cc9531e11b Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Wed, 24 May 2017 12:49:38 +0200 Subject: [PATCH] Use ShareInstance model to access share properties The shares view uses the 'Share' model to access certain properties such as the share type or the share type id. These properties are, however, only proxied to the 'ShareInstance' model. This patch proposes to access these properties through the 'ShareInstance' model directly in order to avoid warning messages for proxied properties which pollute the m-api logs. Closes-Bug: #1692058 Change-Id: I28fa366e106df51c7406673874bd993a44c500bf --- manila/api/views/shares.py | 19 +++++++++++-------- manila/tests/api/contrib/stubs.py | 1 + manila/tests/api/v1/test_shares.py | 7 ++++--- manila/tests/api/v2/test_shares.py | 12 +++++++++--- manila/tests/api/views/test_shares.py | 7 ++++++- manila/tests/fake_share.py | 6 ++++-- 6 files changed, 35 insertions(+), 17 deletions(-) diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index dab6183728..7d51297e5e 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -65,12 +65,13 @@ class ViewBuilder(common.ViewBuilder): export_locations = share.get('export_locations', []) - if share['share_type_id'] and share.get('share_type'): - share_type = share['share_type']['name'] - else: - share_type = share['share_type_id'] - share_instance = share.get('instance') or {} + + if share_instance.get('share_type'): + share_type = share_instance.get('share_type').get('name') + else: + share_type = share_instance.get('share_type_id') + share_dict = { 'id': share.get('id'), 'size': share.get('size'), @@ -109,11 +110,13 @@ class ViewBuilder(common.ViewBuilder): @common.ViewBuilder.versioned_method("2.6") def modify_share_type_field(self, context, share_dict, share): - share_type = share.get('share_type_id') + share_instance = share.get('instance') or {} + + share_type = share_instance.get('share_type_id') share_type_name = None - if share.get('share_type'): - share_type_name = share.get('share_type').get('name') + if share_instance.get('share_type'): + share_type_name = share_instance.get('share_type').get('name') share_dict.update({ 'share_type_name': share_type_name, diff --git a/manila/tests/api/contrib/stubs.py b/manila/tests/api/contrib/stubs.py index 1c517d91a9..7de2782fff 100644 --- a/manila/tests/api/contrib/stubs.py +++ b/manila/tests/api/contrib/stubs.py @@ -54,6 +54,7 @@ def stub_share(id, **kwargs): 'share_network_id': None, 'share_server_id': 'fake_share_server_id', 'access_rules_status': 'active', + 'share_type_id': '1', } if 'instance' in kwargs: share_instance.update(kwargs.pop('instance')) diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index d5c63fc111..5ef01d2723 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -615,9 +615,9 @@ class ShareAPITest(test.TestCase): 'display_name': 'n2', 'status': constants.STATUS_AVAILABLE, 'snapshot_id': 'fake_snapshot_id', - 'share_type_id': 'fake_share_type_id', 'instance': {'host': 'fake_host', - 'share_network_id': 'fake_share_network_id'}, + 'share_network_id': 'fake_share_network_id', + 'share_type_id': 'fake_share_type_id'}, }, {'id': 'id3', 'display_name': 'n3'}, ] @@ -655,7 +655,8 @@ class ShareAPITest(test.TestCase): self.assertEqual( shares[1]['status'], result['shares'][0]['status']) self.assertEqual( - shares[1]['share_type_id'], result['shares'][0]['share_type']) + shares[1]['instance']['share_type_id'], + result['shares'][0]['share_type']) self.assertEqual( shares[1]['snapshot_id'], result['shares'][0]['snapshot_id']) if use_admin_context: diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index 9ac7d8cbff..68cb1b8c6c 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -1529,10 +1529,10 @@ class ShareAPITest(test.TestCase): 'display_name': 'n2', 'status': constants.STATUS_AVAILABLE, 'snapshot_id': 'fake_snapshot_id', - 'share_type_id': 'fake_share_type_id', 'instance': { 'host': 'fake_host', 'share_network_id': 'fake_share_network_id', + 'share_type_id': 'fake_share_type_id', }, }, {'id': 'id3', 'display_name': 'n3'}, @@ -1571,7 +1571,8 @@ class ShareAPITest(test.TestCase): self.assertEqual( shares[1]['status'], result['shares'][0]['status']) self.assertEqual( - shares[1]['share_type_id'], result['shares'][0]['share_type']) + shares[1]['instance']['share_type_id'], + result['shares'][0]['share_type']) self.assertEqual( shares[1]['snapshot_id'], result['shares'][0]['snapshot_id']) if use_admin_context: @@ -2546,7 +2547,12 @@ class ShareManageTest(test.TestCase): } self._setup_manage_mocks() return_share = mock.Mock( - return_value=stubs.stub_share('fake', instance={})) + return_value=stubs.stub_share( + 'fake', + instance={ + 'share_type_id': '1', + }) + ) self.mock_object( share_api.API, 'manage', return_share) share = { diff --git a/manila/tests/api/views/test_shares.py b/manila/tests/api/views/test_shares.py index 4cee082b74..e49f742b34 100644 --- a/manila/tests/api/views/test_shares.py +++ b/manila/tests/api/views/test_shares.py @@ -39,7 +39,12 @@ class ViewBuilderTestCase(test.TestCase): 'export_location': 'fake_export_location', 'export_locations': ['fake_export_location'], 'access_rules_status': 'fake_rule_status', - 'instance': {}, + 'instance': { + 'share_type': { + 'name': 'fake_share_type_name', + }, + 'share_type_id': 'fake_share_type_id', + }, 'replication_type': 'fake_replication_type', 'has_replicas': False, 'user_id': 'fake_userid', diff --git a/manila/tests/fake_share.py b/manila/tests/fake_share.py index ad7d122ab1..91e7ce7056 100644 --- a/manila/tests/fake_share.py +++ b/manila/tests/fake_share.py @@ -31,7 +31,6 @@ def fake_share(**kwargs): 'share_proto': 'fake_proto', 'share_network_id': 'fake share network id', 'share_server_id': 'fake share server id', - 'share_type_id': 'fake share type id', 'export_location': 'fake_location:/fake_share', 'project_id': 'fake_project_uuid', 'availability_zone': 'fake_az', @@ -39,7 +38,10 @@ def fake_share(**kwargs): 'replication_type': None, 'is_busy': False, 'share_group_id': None, - 'instance': {'host': 'fakehost'}, + 'instance': { + 'host': 'fakehost', + 'share_type_id': '1', + }, 'mount_snapshot_support': False, } share.update(kwargs)