Add moref helper functions3.5.0

Since the attribute access on a ManagedObjectReference can differ
depending on the SOAP library used in the backend, we and especially
depending projects should not rely on it, because that breaks the
abstraction layer oslo.vmware is supposed to provide. To help
transitioning to a newer backend library, we introduce two methods for
retrieving a ManagedObjectReference's value and type: get_moref_value()
and get_moref_type().

We cannot handle this by just returning our own ManagedObjectReference
object with uniform access, because it's hard to get control of all
morefs returned by the API.

In that spirit, we also change all references inside of oslo_vmware to
using `get_moref_value()` and `get_moref_type()` instead of directly
accessing the attributes.

This is part of phase 1 of
https://specs.openstack.org/openstack/oslo-specs/specs/victoria/oslo-vmware-soap-library-switch.html

Change-Id: I0b0a3d37f6c7d0c750b48596bc3d79b8b2cb0cee

12 files changed, 97 insertions, 46 deletions

diff --git a/oslo_vmware/api.py b/oslo_vmware/api.py
index 6e18e6d..b286c71 100644
--- a/oslo_vmware/api.py
+++ b/oslo_vmware/api.py
@@ -424,7 +424,7 @@ class VMwareAPISession(object):
                               "task: %s.",
                               task)
         else:
-            task_detail = {'id': task.value}
+            task_detail = {'id': vim_util.get_moref_value(task)}
             # some internal tasks do not have 'name' set
             if getattr(task_info, 'name', None):
                 task_detail['name'] = task_info.name
diff --git a/oslo_vmware/dvs_util.py b/oslo_vmware/dvs_util.py
index 9ee54dd..e0b724d 100644
--- a/oslo_vmware/dvs_util.py
+++ b/oslo_vmware/dvs_util.py
@@ -112,7 +112,7 @@ def add_port_group(session, dvs_moref, name, vlan_id=None,
     task_info = session.wait_for_task(task)
     LOG.info("%(name)s create on %(dvs)s with %(value)s.",
              {'name': name,
-              'dvs': dvs_moref.value,
+              'dvs': vim_util.get_moref_value(dvs_moref),
               'value': task_info.result.value})
     return task_info.result
 
diff --git a/oslo_vmware/pbm.py b/oslo_vmware/pbm.py
index c0a92ab..84c6d3a 100644
--- a/oslo_vmware/pbm.py
+++ b/oslo_vmware/pbm.py
@@ -161,7 +161,7 @@ def convert_datastores_to_hubs(pbm_client_factory, datastores):
     hubs = []
     for ds in datastores:
         hub = pbm_client_factory.create('ns0:PbmPlacementHub')
-        hub.hubId = ds.value
+        hub.hubId = vim_util.get_moref_value(ds)
         hub.hubType = 'Datastore'
         hubs.append(hub)
     return hubs
@@ -177,7 +177,7 @@ def filter_datastores_by_hubs(hubs, datastores):
     filtered_dss = []
     hub_ids = [hub.hubId for hub in hubs]
     for ds in datastores:
-        if ds.value in hub_ids:
+        if vim_util.get_moref_value(ds) in hub_ids:
             filtered_dss.append(ds)
     return filtered_dss
 
@@ -216,7 +216,7 @@ def get_profiles(session, vm):
     profile_manager = pbm.service_content.profileManager
 
     object_ref = pbm.client.factory.create('ns0:PbmServerObjectRef')
-    object_ref.key = vm.value
+    object_ref.key = vim_util.get_moref_value(vm)
     object_ref.objectType = 'virtualMachine'
 
     return session.invoke_api(pbm, 'PbmQueryAssociatedProfile',
diff --git a/oslo_vmware/service.py b/oslo_vmware/service.py
index f61ce60..09702f4 100644
--- a/oslo_vmware/service.py
+++ b/oslo_vmware/service.py
@@ -274,7 +274,8 @@ class Service(object):
                         f_name = f_type.__class__.__name__
                         fault_list.append(f_name)
                         if f_name == exceptions.NO_PERMISSION:
-                            details['object'] = f_type.object.value
+                            details['object'] = \
+                                vim_util.get_moref_value(f_type.object)
                             details['privilegeId'] = f_type.privilegeId
 
         if fault_list:
@@ -349,7 +350,7 @@ class Service(object):
                     op_id = '%s-%s' % (self.op_id_prefix,
                                        uuidutils.generate_uuid())
                     LOG.debug('Invoking %s.%s with opID=%s',
-                              managed_object._type,
+                              vim_util.get_moref_type(managed_object),
                               attr_name,
                               op_id)
                 self._set_soap_headers(op_id)
diff --git a/oslo_vmware/tests/objects/test_datastore.py b/oslo_vmware/tests/objects/test_datastore.py
index 68cc0ea..991c746 100644
--- a/oslo_vmware/tests/objects/test_datastore.py
+++ b/oslo_vmware/tests/objects/test_datastore.py
@@ -179,7 +179,8 @@ class DatastoreClusterTestCase(base.TestCase):
         dsc_moid = 'group-p123'
         dsc_ref, dsc_name = datastore.get_dsc_ref_and_name(session, dsc_moid)
         self.assertEqual((dsc_moid, 'StoragePod'),
-                         (dsc_ref.value, dsc_ref._type))
+                         (vim_util.get_moref_value(dsc_ref),
+                          vim_util.get_moref_type(dsc_ref)))
         self.assertEqual('ds-cluster', dsc_name)
         session.invoke_api.assert_called_once_with(vim_util,
                                                    'get_object_property',
@@ -205,8 +206,10 @@ class DatastoreClusterTestCase(base.TestCase):
         session.invoke_api.return_value = retrieve_result
         name = 'ds-cluster'
         dsc_ref, dsc_name = datastore.get_dsc_ref_and_name(session, name)
-        self.assertEqual((pod_ref.value, pod_ref._type),
-                         (dsc_ref.value, dsc_ref._type))
+        self.assertEqual((vim_util.get_moref_value(pod_ref),
+                          vim_util.get_moref_type(pod_ref)),
+                         (vim_util.get_moref_value(dsc_ref),
+                          vim_util.get_moref_type(dsc_ref)))
 
 
 class DatastorePathTestCase(base.TestCase):
diff --git a/oslo_vmware/tests/test_api.py b/oslo_vmware/tests/test_api.py
index 499436d..af2a42d 100644
--- a/oslo_vmware/tests/test_api.py
+++ b/oslo_vmware/tests/test_api.py
@@ -587,8 +587,7 @@ class VMwareAPISessionTest(base.TestCase):
         with (
             mock.patch.object(api_session, 'invoke_api', fake_invoke_api)
         ):
-            fake_task = mock.Mock()
-            fake_task.value = 'task-1'
+            fake_task = vim_util.get_moref('Task', 'task-1')
             ctx = mock.Mock()
             self.assertRaises(expected_exception,
                               api_session._poll_task,
diff --git a/oslo_vmware/tests/test_dvs_util.py b/oslo_vmware/tests/test_dvs_util.py
index 8382a40..6403a47 100644
--- a/oslo_vmware/tests/test_dvs_util.py
+++ b/oslo_vmware/tests/test_dvs_util.py
@@ -32,8 +32,9 @@ class DvsUtilTest(base.TestCase):
 
     def test_get_dvs_moref(self):
         moref = dvs_util.get_dvs_moref('dvs-123')
-        self.assertEqual('dvs-123', moref.value)
-        self.assertEqual('VmwareDistributedVirtualSwitch', moref._type)
+        self.assertEqual('dvs-123', vim_util.get_moref_value(moref))
+        self.assertEqual('VmwareDistributedVirtualSwitch',
+                         vim_util.get_moref_type(moref))
 
     def test_get_vlan_spec(self):
         session = mock.Mock()
diff --git a/oslo_vmware/tests/test_pbm.py b/oslo_vmware/tests/test_pbm.py
index 2a3b29f..e4dcd10 100644
--- a/oslo_vmware/tests/test_pbm.py
+++ b/oslo_vmware/tests/test_pbm.py
@@ -23,6 +23,7 @@ import urllib.request as urllib
 
 from oslo_vmware import pbm
 from oslo_vmware.tests import base
+from oslo_vmware import vim_util
 
 
 class PBMUtilityTest(base.TestCase):
@@ -108,8 +109,7 @@ class PBMUtilityTest(base.TestCase):
             profile=profile_id)
 
     def _create_datastore(self, value):
-        ds = mock.Mock()
-        ds.value = value
+        ds = vim_util.get_moref(value, 'Datastore')
         return ds
 
     def test_convert_datastores_to_hubs(self):
@@ -144,7 +144,8 @@ class PBMUtilityTest(base.TestCase):
 
         filtered_ds = pbm.filter_datastores_by_hubs(hubs, datastores)
         self.assertEqual(len(hubs), len(filtered_ds))
-        filtered_ds_values = [ds.value for ds in filtered_ds]
+        filtered_ds_values = [vim_util.get_moref_value(ds)
+                              for ds in filtered_ds]
         self.assertEqual(set(hub_ids), set(filtered_ds_values))
 
     def test_get_pbm_wsdl_location(self):
@@ -180,7 +181,7 @@ class PBMUtilityTest(base.TestCase):
         session.invoke_api.return_value = profile_id
 
         value = 'vm-1'
-        vm = mock.Mock(value=value)
+        vm = vim_util.get_moref(value, 'VirtualMachine')
         ret = pbm.get_profiles(session, vm)
 
         self.assertEqual(profile_id, ret)
diff --git a/oslo_vmware/tests/test_service.py b/oslo_vmware/tests/test_service.py
index c3cc23f..8fc9945 100644
--- a/oslo_vmware/tests/test_service.py
+++ b/oslo_vmware/tests/test_service.py
@@ -95,8 +95,8 @@ class ServiceTest(base.TestCase):
         resp = mock.Mock()
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             return resp
 
         svc_obj = service.Service()
@@ -110,8 +110,8 @@ class ServiceTest(base.TestCase):
         managed_object = 'Datacenter'
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             return None
 
         svc_obj = service.Service()
@@ -129,8 +129,8 @@ class ServiceTest(base.TestCase):
         doc = mock.Mock()
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             fault = mock.Mock(faultstring="MyFault")
 
             fault_children = mock.Mock()
@@ -180,8 +180,8 @@ class ServiceTest(base.TestCase):
         doc = mock.Mock()
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             fault = mock.Mock(faultstring="MyFault")
 
             fault_children = mock.Mock()
@@ -215,8 +215,8 @@ class ServiceTest(base.TestCase):
         doc = mock.Mock()
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             fault = mock.Mock(faultstring="MyFault")
 
             fault_children = mock.Mock()
@@ -257,8 +257,8 @@ class ServiceTest(base.TestCase):
         managed_object = 'VirtualMachine'
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             raise httplib.CannotSendRequest()
 
         svc_obj = service.Service()
@@ -273,8 +273,8 @@ class ServiceTest(base.TestCase):
         managed_object = 'VirtualMachine'
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             raise httplib.ResponseNotReady()
 
         svc_obj = service.Service()
@@ -289,8 +289,8 @@ class ServiceTest(base.TestCase):
         managed_object = 'VirtualMachine'
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             raise httplib.CannotSendHeader()
 
         svc_obj = service.Service()
@@ -305,8 +305,8 @@ class ServiceTest(base.TestCase):
         managed_object = 'VirtualMachine'
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             raise requests.ConnectionError()
 
         svc_obj = service.Service()
@@ -321,8 +321,8 @@ class ServiceTest(base.TestCase):
         managed_object = 'VirtualMachine'
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             raise requests.HTTPError()
 
         svc_obj = service.Service()
@@ -344,8 +344,8 @@ class ServiceTest(base.TestCase):
         managed_object = 'VirtualMachine'
 
         def side_effect(mo, **kwargs):
-            self.assertEqual(managed_object, mo._type)
-            self.assertEqual(managed_object, mo.value)
+            self.assertEqual(managed_object, vim_util.get_moref_type(mo))
+            self.assertEqual(managed_object, vim_util.get_moref_value(mo))
             raise Exception(message)
 
         svc_obj = service.Service()
diff --git a/oslo_vmware/tests/test_vim_util.py b/oslo_vmware/tests/test_vim_util.py
index e73bf6e..43c02bc 100644
--- a/oslo_vmware/tests/test_vim_util.py
+++ b/oslo_vmware/tests/test_vim_util.py
@@ -27,8 +27,8 @@ class VimUtilTest(base.TestCase):
 
     def test_get_moref(self):
         moref = vim_util.get_moref("vm-0", "VirtualMachine")
-        self.assertEqual("vm-0", moref.value)
-        self.assertEqual("VirtualMachine", moref._type)
+        self.assertEqual("vm-0", vim_util.get_moref_value(moref))
+        self.assertEqual("VirtualMachine", vim_util.get_moref_type(moref))
 
     def test_build_selection_spec(self):
         client_factory = mock.Mock()
@@ -214,8 +214,7 @@ class VimUtilTest(base.TestCase):
     @mock.patch('oslo_vmware.vim_util.cancel_retrieval')
     def test_get_object_properties(self, cancel_retrieval):
         vim = mock.Mock()
-        moref = mock.Mock()
-        moref._type = "VirtualMachine"
+        moref = vim_util.get_moref('fake-ref', 'VirtualMachine')
         retrieve_result = mock.Mock()
 
         def vim_RetrievePropertiesEx_side_effect(pc, specSet, options,
@@ -231,7 +230,7 @@ class VimUtilTest(base.TestCase):
             prop_spec = propSet[0]
             self.assertTrue(prop_spec.all)
             self.assertEqual(['name'], prop_spec.pathSet)
-            self.assertEqual(moref._type, prop_spec.type)
+            self.assertEqual(vim_util.get_moref_type(moref), prop_spec.type)
 
             objSet = property_filter_spec.objectSet
             self.assertEqual(1, len(objSet))
diff --git a/oslo_vmware/vim_util.py b/oslo_vmware/vim_util.py
index 85a7fd8..b2c9d2c 100644
--- a/oslo_vmware/vim_util.py
+++ b/oslo_vmware/vim_util.py
@@ -37,6 +37,44 @@ def get_moref(value, type_):
     return moref
 
 
+def get_moref_value(moref):
+    """Get the value/id of a managed object reference
+
+    This function accepts a string representation of a ManagedObjectReference
+    like `VirtualMachine:vm-123` or only `vm-123`, but is also able to extract
+    it from the actual object as returned by the API.
+    """
+    if isinstance(moref, str):
+        # handle strings like VirtualMachine:vm-12312, but also vm-123123
+        if ':' in moref:
+            splits = moref.split(':')
+            return splits[1]
+        return moref
+
+    # assume it's a ManagedObjectReference object as created by `get_moref()`
+    # or returned by a request
+    return moref.value
+
+
+def get_moref_type(moref):
+    """Get the type of a managed object reference
+
+    This function accepts a string representation of a ManagedObjectReference
+    like `VirtualMachine:vm-123`, but is also able to extract it from the
+    actual object as returned by the API.
+    """
+    if isinstance(moref, str):
+        # handle strings like VirtualMachine:vm-12312
+        if ':' in moref:
+            splits = moref.split(':')
+            return splits[0]
+        return None
+
+    # assume it's a ManagedObjectReference object as created by `get_moref()`
+    # or returned by a request
+    return moref._type
+
+
 def build_selection_spec(client_factory, name):
     """Builds the selection spec.
 
@@ -309,7 +347,7 @@ def get_object_properties(vim, moref, properties_to_collect, skip_op_id=False):
                       len(properties_to_collect) == 0)
     property_spec = build_property_spec(
         client_factory,
-        type_=moref._type,
+        type_=get_moref_type(moref),
         properties_to_collect=properties_to_collect,
         all_properties=all_properties)
     object_spec = build_object_spec(client_factory, moref, [])
diff --git a/releasenotes/notes/bp-oslo-vmware-soap-library-switch-helper-functions-21330be61cf7922a.yaml b/releasenotes/notes/bp-oslo-vmware-soap-library-switch-helper-functions-21330be61cf7922a.yaml
new file mode 100644
index 0000000..f2ef8d2
--- /dev/null
+++ b/releasenotes/notes/bp-oslo-vmware-soap-library-switch-helper-functions-21330be61cf7922a.yaml
@@ -0,0 +1,9 @@
+---
+upgrade:
+  - |
+    ``ManagedObjectReference``'s ``value`` and ``_type`` attributes must not be
+    used anymore in depending projects. Instead, ``vim_util.get_moref_value()``
+    and ``vim_util.get_moref_type()`` have to be used, because with
+    `this spec <https://specs.openstack.org/openstack/oslo-specs/specs/victoria/oslo-vmware-soap-library-switch.html>`_
+    we switch the backing SOAP library and different libraries have different
+    representations of those attributes.