Move and generalize decorator serialize_args to nova.objects.base

It would be nice use decorator `serialize_args` as general-use
decorator. Note that this single-purpose decorator was covering
up our ability to drill down to the actual method implementation
for hash calculation in the TestObjectVersions test. This patch
also enhances the decorator to be transparent, and makes the
test able to drill down through multiple layers of decorators
to find the actual implementation.

Co-Authored-By: Dan Smith <dansmith@redhat.com>
Change-Id: Ie454bffba113d7b3a863e975b2fb0ee6fb1dd552

nova/objects/base.py

@@ -17,6 +17,7 @@
 import collections
 import copy
 import functools
+import traceback
 
 import netaddr
 from oslo import messaging
@@ -704,3 +705,22 @@ def obj_make_list(context, list_obj, item_cls, db_list, **extra_args):
     list_obj._context = context
     list_obj.obj_reset_changes()
     return list_obj
+
+
+def serialize_args(fn):
+    """Decorator that will do the arguments serialization before remoting."""
+    def wrapper(cls, *args, **kwargs):
+        for kw in kwargs:
+            value_arg = kwargs.get(kw)
+            if kw == 'exc_val' and value_arg:
+                kwargs[kw] = str(value_arg)
+            if kw == 'exc_tb' and (
+                    not isinstance(value_arg, six.string_types) and value_arg):
+                kwargs[kw] = ''.join(traceback.format_tb(value_arg))
+        # NOTE(danms): We wrap a descriptor, so use that protocol
+        return fn.__get__(None, cls)(*args, **kwargs)
+
+    # NOTE(danms): Make this discoverable
+    wrapper.remotable = getattr(fn, 'remotable', False)
+    wrapper.original_fn = fn
+    return classmethod(wrapper)

nova/objects/instance_action.py

@@ -12,10 +12,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import traceback
-
-import six
-
 from nova import db
 from nova import objects
 from nova.objects import base
@@ -112,19 +108,6 @@ class InstanceActionList(base.ObjectListBase, base.NovaObject):
         return base.obj_make_list(context, cls(), InstanceAction, db_actions)
 
 
-def serialize_args(fn):
-    def wrapper(cls, *args, **kwargs):
-        exc_val = kwargs.get('exc_val')
-        exc_tb = kwargs.get('exc_tb')
-        if exc_val is not None:
-            kwargs['exc_val'] = six.text_type(exc_val)
-        if not isinstance(exc_tb, six.string_types) and exc_tb is not None:
-            kwargs['exc_tb'] = ''.join(traceback.format_tb(exc_tb))
-        # NOTE(danms): We wrap a descriptor, so use that protocol
-        return fn.__get__(None, cls)(*args, **kwargs)
-    return classmethod(wrapper)
-
-
 class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject):
     # Version 1.0: Initial version
     # Version 1.1: event_finish_with_failure decorated with serialize_args
@@ -183,7 +166,7 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject):
         if want_result:
             return cls._from_db_object(context, cls(), db_event)
 
-    @serialize_args
+    @base.serialize_args
     @base.remotable_classmethod
     def event_finish_with_failure(cls, context, instance_uuid, event_name,
                                   exc_val=None, exc_tb=None, want_result=None):

nova/tests/objects/test_objects.py

@@ -950,7 +950,7 @@ object_data = {
     'FloatingIPList': '1.2-6c5b0b4d4a4c17575f4d91bae14e5237',
     'Instance': '1.13-c9cfd71ddc9d6e7e7c72879f4d5982ee',
     'InstanceAction': '1.1-6b1d0a6dbd522b5a83c20757ec659663',
-    'InstanceActionEvent': '1.1-f144eaa9fb22f248fc41ed8401a3a1be',
+    'InstanceActionEvent': '1.1-42dbdba74bd06e0619ca75cd3397cd1b',
     'InstanceActionEventList': '1.0-1d5cc958171d6ce07383c2ad6208318e',
     'InstanceActionList': '1.0-368410fdb8d69ae20c495308535d6266',
     'InstanceExternalEvent': '1.0-f1134523654407a875fd59b80f759ee7',
@@ -1005,6 +1005,21 @@ class TestObjectVersions(test.TestCase):
     def setUp(self):
         super(TestObjectVersions, self).setUp()
 
+    def _find_remotable_method(self, cls, thing, parent_was_remotable=False):
+        """Follow a chain of remotable things down to the original function."""
+        if isinstance(thing, classmethod):
+            return self._find_remotable_method(cls, thing.__get__(None, cls))
+        elif inspect.ismethod(thing) and hasattr(thing, 'remotable'):
+            return self._find_remotable_method(cls, thing.original_fn,
+                                               parent_was_remotable=True)
+        elif parent_was_remotable:
+            # We must be the first non-remotable thing underneath a stack of
+            # remotable things (i.e. the actual implementation method)
+            return thing
+        else:
+            # This means the top-level thing never hit a remotable layer
+            return None
+
     def _get_fingerprint(self, obj_name):
         obj_class = base.NovaObject._obj_classes[obj_name][0]
         fields = obj_class.fields.items()
@@ -1012,8 +1027,10 @@ class TestObjectVersions(test.TestCase):
         methods = []
         for name in dir(obj_class):
             thing = getattr(obj_class, name)
-            if inspect.ismethod(thing) and hasattr(thing, 'remotable'):
-                methods.append((name, inspect.getargspec(thing.original_fn)))
+            if inspect.ismethod(thing) or isinstance(thing, classmethod):
+                method = self._find_remotable_method(obj_class, thing)
+                if method:
+                    methods.append((name, inspect.getargspec(method)))
         methods.sort()
         # NOTE(danms): Things that need a version bump are any fields
         # and their types, or the signatures of any remotable methods.