Merge "Fix remotable object change tracking"

oslo_versionedobjects/base.py

@@ -599,8 +599,13 @@ class VersionedObject(object):
                self._obj_primitive_key('version'): target_version,
                self._obj_primitive_key('data'): primitive}
         if self.obj_what_changed():
-            obj[self._obj_primitive_key('changes')] = list(
-                self.obj_what_changed())
+            # NOTE(cfriesen): if we're downgrading to a lower version, then
+            # it's possible that self.obj_what_changed() includes fields that
+            # no longer exist in the lower version.  If so, filter them out.
+            what_changed = self.obj_what_changed()
+            changes = [field for field in what_changed if field in primitive]
+            if changes:
+                obj[self._obj_primitive_key('changes')] = changes
         return obj
 
     def obj_set_defaults(self, *attrs):
@@ -636,7 +641,8 @@ class VersionedObject(object):
 
     def obj_what_changed(self):
         """Returns a set of fields that have been modified."""
-        changes = set(self._changed_fields)
+        changes = set([field for field in self._changed_fields
+                       if field in self.fields])
         for field in self.fields:
             if (self.obj_attr_is_set(field) and
                     isinstance(getattr(self, field), VersionedObject) and

oslo_versionedobjects/tests/test_fixture.py

@@ -708,11 +708,15 @@ class TestVersionedObjectRegistryFixture(test.TestCase):
 
 class TestStableObjectJsonFixture(test.TestCase):
     def test_changes_sort(self):
+        @base.VersionedObjectRegistry.register_if(False)
         class TestObject(base.VersionedObject):
+            fields = {'z': fields.StringField(),
+                      'a': fields.StringField()}
+
             def obj_what_changed(self):
                 return ['z', 'a']
 
-        obj = TestObject()
+        obj = TestObject(a='foo', z='bar')
         self.assertEqual(['z', 'a'],
                          obj.obj_to_primitive()['versioned_object.changes'])
         with fixture.StableObjectJsonFixture():

oslo_versionedobjects/tests/test_objects.py

@@ -994,6 +994,15 @@ class _TestObject(object):
         bar.foo = 1
         self.assertEqual(set(['bar']), obj.obj_what_changed())
 
+    def test_changed_with_bogus_field(self):
+        obj = MyObj()
+        obj.foo = 123
+        # Add a bogus field name to the changed list, as could be the
+        # case if we're sent some broken primitive from another node.
+        obj._changed_fields.add('does_not_exist')
+        self.assertEqual(set(['foo']), obj.obj_what_changed())
+        self.assertEqual({'foo': 123}, obj.obj_get_changes())
+
     def test_static_result(self):
         obj = MyObj.query(self.context)
         self.assertEqual(obj.bar, 'bar')
@@ -1334,6 +1343,20 @@ class _TestObject(object):
                 primitive['rel_objects'][0]['versioned_object.data'],
                 '1.2', version_manifest=manifest)
 
+    def test_obj_make_compatible_removes_field_cleans_changes(self):
+        @base.VersionedObjectRegistry.register_if(False)
+        class TestObject(base.VersionedObject):
+            VERSION = '1.1'
+            fields = {'foo': fields.StringField(),
+                      'bar': fields.StringField()}
+
+            def obj_make_compatible(self, primitive, target_version):
+                del primitive['bar']
+
+        obj = TestObject(foo='test1', bar='test2')
+        prim = obj.obj_to_primitive('1.0')
+        self.assertEqual(['foo'], prim['versioned_object.changes'])
+
     def test_delattr(self):
         obj = MyObj(bar='foo')
         del obj.bar