From 6428a3cd1f6b00292c0ebe18e4ca7b7ddfc3f9c3 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 (cherry picked from commit 2e6969c3fe59184e5ae8bb9d327201cc9531e11b) --- 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 572baf2ba1..c5392bb0d7 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 72be15f45d..9701b4936e 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: @@ -2538,7 +2539,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)