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 2e6969c3fe)

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'):
+        if share_instance.get('share_type'):
-            share_type_name = share.get('share_type').get('name')
+            share_type_name = share_instance.get('share_type').get('name')
 
         share_dict.update({
             'share_type_name': share_type_name,

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'))

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:

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 = {

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',

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)