Provide stable getargspec() behavior for method fingerprinting

Python3 has deprecated inspect.getargspec(), which our fixture uses
to provide fingerprinting for remotable methods. Since the hashesh for
those methods are stored in the wild and are used to detect when objects
change in incompatible ways, it is desirable to avoid having to change
all stored hashes when a library adopts the newer method.

This change attempts to use the older spec format when possible to
avoid needless hash changes, and opts for the newer one when necessary
to represent some newer feature. Changing hashes when adding such a feature
to a remotable method is implied anyway, so getting the newer one based
on the newer method is expected.

Change-Id: I84b4ce9c95d6ab86c58f8d797dba28201c1f1668

oslo_versionedobjects/fixture.py

@@ -20,6 +20,7 @@
 
 """
 
+from collections import namedtuple
 from collections import OrderedDict
 import copy
 import datetime
@@ -196,6 +197,31 @@ class ObjectHashMismatch(Exception):
             ','.join(set(self.expected.keys() + self.actual.keys())))
 
 
+CompatArgSpec = namedtuple(
+    'ArgSpec', ('args', 'varargs', 'keywords', 'defaults'))
+
+
+def get_method_spec(method):
+    """Get a stable and compatible method spec.
+
+    Newer features in Python3 (kw-only arguments and annotations) are
+    not supported or representable with inspect.getargspec() but many
+    object hashes are already recorded using that method. This attempts
+    to return something compatible with getargspec() when possible (i.e.
+    when those features are not used), and otherwise just returns the
+    newer getfullargspec() representation.
+    """
+    fullspec = inspect.getfullargspec(method)
+    if any([fullspec.kwonlyargs, fullspec.kwonlydefaults,
+            fullspec.annotations]):
+        # Method uses newer-than-getargspec() features, so return the
+        # newer full spec
+        return fullspec
+    else:
+        return CompatArgSpec(fullspec.args, fullspec.varargs,
+                             fullspec.varkw, fullspec.defaults)
+
+
 class ObjectVersionChecker(object):
     def __init__(self, obj_classes=base.VersionedObjectRegistry.obj_classes()):
         self.obj_classes = obj_classes
@@ -227,7 +253,7 @@ class ObjectVersionChecker(object):
                or isinstance(thing, classmethod):
                 method = self._find_remotable_method(obj_class, thing)
                 if method:
-                    methods.append((name, inspect.getargspec(method)))
+                    methods.append((name, get_method_spec(method)))
         methods.sort()
         # NOTE(danms): Things that need a version bump are any fields
         # and their types, or the signatures of any remotable methods.

oslo_versionedobjects/tests/test_fixture.py

@@ -16,6 +16,7 @@ import collections
 import copy
 import datetime
 import hashlib
+import inspect
 
 import iso8601
 import mock
@@ -529,7 +530,7 @@ class TestObjectVersionChecker(test.TestCase):
         MyObject.VERSION = '1.1'
         argspec = 'vulpix'
 
-        with mock.patch('inspect.getargspec') as mock_gas:
+        with mock.patch.object(fixture, 'get_method_spec') as mock_gas:
             mock_gas.return_value = argspec
             fp = self.ovc._get_fingerprint(MyObject.__name__)
 
@@ -552,7 +553,7 @@ class TestObjectVersionChecker(test.TestCase):
         MyObject.child_versions = child_versions
         argspec = 'onix'
 
-        with mock.patch('inspect.getargspec') as mock_gas:
+        with mock.patch.object(fixture, 'get_method_spec') as mock_gas:
             mock_gas.return_value = argspec
             fp = self.ovc._get_fingerprint(MyObject.__name__)
 
@@ -736,3 +737,29 @@ class TestStableObjectJsonFixture(test.TestCase):
             self.assertEqual(
                 ['a', 'z'],
                 obj.obj_to_primitive()['versioned_object.changes'])
+
+
+class TestMethodSpec(test.TestCase):
+    def setUp(self):
+        super(TestMethodSpec, self).setUp()
+
+        def test_method1(a, b, kw1=123, **kwargs):
+            pass
+
+        def test_method2(a, b, *args):
+            pass
+
+        def test_method3(a, b, *args, kw1=123, **kwargs):
+            pass
+
+        self._test_method1 = test_method1
+        self._test_method2 = test_method2
+        self._test_method3 = test_method3
+
+    def test_method_spec_compat(self):
+        self.assertEqual(inspect.getargspec(self._test_method1),
+                         fixture.get_method_spec(self._test_method1))
+        self.assertEqual(inspect.getargspec(self._test_method2),
+                         fixture.get_method_spec(self._test_method2))
+        self.assertEqual(inspect.getfullargspec(self._test_method3),
+                         fixture.get_method_spec(self._test_method3))