Address issues with objects of same name

There are at least a couple of cases where we may (in the future) introduce
multiple objects that have the same name:

1) When we subclass an object to override internal behavior, presenting
the same version of the object.

2) When we want to introduce multiple versions of objects either via
subclassing or completely separate classes if differences are large
enough to warrant that.

There was a rough plan to support this, however we're currently very
broken. The code before this patch would happily track every one of the
objects with the same name. However, in the case where we see the same
version during 'object registration', we should actually always take the
newest one that we see, replacing the old one.

Also, throughout nova, we have code such as:

from nova.objects import instance as instance_obj

inst = instance_obj.Instance.<....>

This means we're always using the base object, which may or may not be
the correct ('newest') one.

This patch fixes the object registration and adds an attribute to the
'nova.objects' module for every object that is registered, setting the
attribute to the newest version we see.

This will allow us to drop a number of object module imports in favor
of simply importing nova.objects. The example above would become:

from nova import objects

inst = objects.Instance.<....>

Because the object registration ensures objects.Instance is the newest
version, we'll be golden.

Partial-Blueprint: object-subclassing

Change-Id: I04694ecd40e0d4ec40a87d319f2e689060b11651

nova/objects/base.py

@@ -24,6 +24,7 @@
 
 from nova import context
 from nova import exception
+from nova import objects
 from nova.objects import fields
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import log as logging
@@ -89,12 +90,40 @@ class NovaObjectMetaclass(type):
 
     def __init__(cls, names, bases, dict_):
         if not hasattr(cls, '_obj_classes'):
-            # This will be set in the 'NovaObject' class.
+            # This means this is a base class using the metaclass. I.e.,
+            # the 'NovaObject' class.
             cls._obj_classes = collections.defaultdict(list)
+            return
+
+        def _vers_tuple(obj):
+            return tuple([int(x) for x in obj.VERSION.split(".")])
+
+        # Add the subclass to NovaObject._obj_classes. If the
+        # same version already exists, replace it. Otherwise,
+        # keep the list with newest version first.
+        make_class_properties(cls)
+        obj_name = cls.obj_name()
+        for i, obj in enumerate(cls._obj_classes[obj_name]):
+            if cls.VERSION == obj.VERSION:
+                cls._obj_classes[obj_name][i] = cls
+                # Update nova.objects with this newer class.
+                setattr(objects, obj_name, cls)
+                break
+            if _vers_tuple(cls) > _vers_tuple(obj):
+                # Insert before.
+                cls._obj_classes[obj_name].insert(i, cls)
+                if i == 0:
+                    # Later version than we've seen before. Update
+                    # nova.objects.
+                    setattr(objects, obj_name, cls)
+                break
         else:
-            # Add the subclass to NovaObject._obj_classes
-            make_class_properties(cls)
-            cls._obj_classes[cls.obj_name()].append(cls)
+            cls._obj_classes[obj_name].append(cls)
+            # Either this is the first time we've seen the object or it's
+            # an older version than anything we'e seen. Update nova.objects
+            # only if it's the first time we've seen this object name.
+            if not hasattr(objects, obj_name):
+                setattr(objects, obj_name, cls)
 
 
 # These are decorators that mark an object's method as remotable.
@@ -203,25 +232,24 @@ def obj_class_from_name(cls, objname, objver):
                         '%(objtype)s') % dict(objtype=objname))
             raise exception.UnsupportedObjectError(objtype=objname)
 
-        latest = None
+        # NOTE(comstud): If there's not an exact match, return the highest
+        # compatible version. The objects stored in the class are sorted
+        # such that highest version is first, so only set compatible_match
+        # once below.
         compatible_match = None
+
         for objclass in cls._obj_classes[objname]:
             if objclass.VERSION == objver:
                 return objclass
-
-            version_bits = tuple([int(x) for x in objclass.VERSION.split(".")])
-            if latest is None:
-                latest = version_bits
-            elif latest < version_bits:
-                latest = version_bits
-
-            if versionutils.is_compatible(objver, objclass.VERSION):
+            if (not compatible_match and
+                    versionutils.is_compatible(objver, objclass.VERSION)):
                 compatible_match = objclass
 
         if compatible_match:
             return compatible_match
 
-        latest_ver = '%i.%i' % latest
+        # As mentioned above, latest version is always first in the list.
+        latest_ver = cls._obj_classes[objname][0].VERSION
         raise exception.IncompatibleObjectVersion(objname=objname,
                                                   objver=objver,
                                                   supported=latest_ver)

nova/tests/objects/test_objects.py

@@ -22,6 +22,7 @@
 from nova.conductor import rpcapi as conductor_rpcapi
 from nova import context
 from nova import exception
+from nova import objects
 from nova.objects import base
 from nova.objects import fields
 from nova.openstack.common import jsonutils
@@ -88,6 +89,14 @@ def obj_make_compatible(self, primitive, target_version):
             primitive['bar'] = 'old%s' % primitive['bar']
 
 
+class MyObjDiffVers(MyObj):
+    VERSION = '1.4'
+
+    @classmethod
+    def obj_name(cls):
+        return 'MyObj'
+
+
 class MyObj2(object):
     @classmethod
     def obj_name(cls):
@@ -112,31 +121,51 @@ def test_obj_tracking(self):
 
         @six.add_metaclass(base.NovaObjectMetaclass)
         class NewBaseClass(object):
+            VERSION = '1.0'
             fields = {}
 
             @classmethod
             def obj_name(cls):
                 return cls.__name__
 
-        class Test1(NewBaseClass):
-            @staticmethod
-            def obj_name():
+        class Fake1TestObj1(NewBaseClass):
+            @classmethod
+            def obj_name(cls):
                 return 'fake1'
 
-        class Test2(NewBaseClass):
+        class Fake1TestObj2(Fake1TestObj1):
             pass
 
-        class Test2v2(NewBaseClass):
-            @staticmethod
-            def obj_name():
-                return 'Test2'
+        class Fake1TestObj3(Fake1TestObj1):
+            VERSION = '1.1'
 
-        expected = {'fake1': [Test1], 'Test2': [Test2, Test2v2]}
+        class Fake2TestObj1(NewBaseClass):
+            @classmethod
+            def obj_name(cls):
+                return 'fake2'
+
+        class Fake1TestObj4(Fake1TestObj3):
+            VERSION = '1.2'
+
+        class Fake2TestObj2(Fake2TestObj1):
+            VERSION = '1.1'
 
+        class Fake1TestObj5(Fake1TestObj1):
+            VERSION = '1.1'
+
+        # Newest versions first in the list. Duplicate versions take the
+        # newest object.
+        expected = {'fake1': [Fake1TestObj4, Fake1TestObj5, Fake1TestObj2],
+                    'fake2': [Fake2TestObj2, Fake2TestObj1]}
         self.assertEqual(expected, NewBaseClass._obj_classes)
         # The following should work, also.
-        self.assertEqual(expected, Test1._obj_classes)
-        self.assertEqual(expected, Test2._obj_classes)
+        self.assertEqual(expected, Fake1TestObj1._obj_classes)
+        self.assertEqual(expected, Fake1TestObj2._obj_classes)
+        self.assertEqual(expected, Fake1TestObj3._obj_classes)
+        self.assertEqual(expected, Fake1TestObj4._obj_classes)
+        self.assertEqual(expected, Fake1TestObj5._obj_classes)
+        self.assertEqual(expected, Fake2TestObj1._obj_classes)
+        self.assertEqual(expected, Fake2TestObj2._obj_classes)
 
     def test_field_checking(self):
         def create_class(field):
@@ -145,7 +174,7 @@ class TestField(base.NovaObject):
                 fields = {'foo': field()}
             return TestField
 
-        cls = create_class(fields.IPV4AndV6AddressField)
+        create_class(fields.IPV4AndV6AddressField)
         self.assertRaises(exception.ObjectFieldInvalid,
                           create_class, fields.IPV4AndV6Address)
         self.assertRaises(exception.ObjectFieldInvalid,
@@ -349,6 +378,14 @@ def assertRemotes(self):
 
 
 class _TestObject(object):
+    def test_object_attrs_in_init(self):
+        # Spot check a few
+        objects.Instance
+        objects.InstanceInfoCache
+        objects.SecurityGroup
+        # Now check the test one in this file. Should be newest version
+        self.assertEqual('1.5', objects.MyObj.VERSION)
+
     def test_hydration_type_error(self):
         primitive = {'nova_object.name': 'MyObj',
                      'nova_object.namespace': 'nova',
@@ -456,6 +493,14 @@ def test_changes_in_primitive(self):
         obj2.obj_reset_changes()
         self.assertEqual(obj2.obj_what_changed(), set())
 
+    def test_obj_class_from_name(self):
+        obj = base.NovaObject.obj_class_from_name('MyObj', '1.4')
+        self.assertEqual('1.4', obj.VERSION)
+
+    def test_obj_class_from_name_latest_compatible(self):
+        obj = base.NovaObject.obj_class_from_name('MyObj', '1.1')
+        self.assertEqual('1.5', obj.VERSION)
+
     def test_unknown_objtype(self):
         self.assertRaises(exception.UnsupportedObjectError,
                           base.NovaObject.obj_class_from_name, 'foo', '1.0')