Make NovaObject report changed-ness of its children

This makes NovaObject implementations report the changed-ness of
their child objects. This avoids the case where a child object is
touched and the parent doesn't notice that it needs to be saved
back to the database.

Change-Id: Iabda4206b3662b46dab70c925c661fdedeebc432

nova/objects/base.py

@@ -328,7 +328,13 @@ class NovaObject(object):
 
     def obj_what_changed(self):
         """Returns a set of fields that have been modified."""
-        return self._changed_fields
+        changes = set(self._changed_fields)
+        for field in self.fields:
+            if (self.obj_attr_is_set(field) and
+                    isinstance(self[field], NovaObject) and
+                    self[field].obj_what_changed()):
+                changes.add(field)
+        return changes
 
     def obj_get_changes(self):
         """Returns a dict of changed fields and their new values."""
@@ -507,6 +513,13 @@ class ObjectListBase(object):
                 child_target_version)
             primitives[index]['nova_object.version'] = child_target_version
 
+    def obj_what_changed(self):
+        changes = set(self._changed_fields)
+        for child in self.objects:
+            if child.obj_what_changed():
+                changes.add('objects')
+        return changes
+
 
 class NovaObjectSerializer(messaging.NoOpSerializer):
     """A NovaObject-aware Serializer.

nova/objects/instance.py

@@ -372,6 +372,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
     def _save_security_groups(self, context):
         for secgroup in self.security_groups:
             secgroup.save(context)
+        self.security_groups.obj_reset_changes()
 
     def _save_fault(self, context):
         # NOTE(danms): I don't think we need to worry about this, do we?

nova/tests/objects/test_instance.py

@@ -91,8 +91,8 @@ class _TestInstanceObject(object):
                         {'uuid': 'fake-uuid',
                          'access_ip_v4': '1.2.3.4',
                          'access_ip_v6': '::1'},
-                    'nova_object.changes': ['access_ip_v4', 'uuid',
-                                            'access_ip_v6']}
+                    'nova_object.changes': ['uuid', 'access_ip_v6',
+                                            'access_ip_v4']}
         self.assertEqual(primitive, expected)
         inst2 = instance.Instance.obj_from_primitive(primitive)
         self.assertIsInstance(inst2.access_ip_v4, netaddr.IPAddress)

nova/tests/objects/test_objects.py

@@ -605,6 +605,23 @@ class _TestObject(object):
         self.assertEqual(obj.bar, 'meow')
         self.assertRemotes()
 
+    def test_changed_with_sub_object(self):
+        class ParentObject(base.NovaObject):
+            fields = {'foo': fields.IntegerField(),
+                      'bar': fields.ObjectField('MyObj'),
+                      }
+        obj = ParentObject()
+        self.assertEqual(set(), obj.obj_what_changed())
+        obj.foo = 1
+        self.assertEqual(set(['foo']), obj.obj_what_changed())
+        bar = MyObj()
+        obj.bar = bar
+        self.assertEqual(set(['foo', 'bar']), obj.obj_what_changed())
+        obj.obj_reset_changes()
+        self.assertEqual(set(), obj.obj_what_changed())
+        bar.foo = 1
+        self.assertEqual(set(['bar']), obj.obj_what_changed())
+
     def test_static_result(self):
         obj = MyObj.query(self.context)
         self.assertEqual(obj.bar, 'bar')
@@ -799,6 +816,24 @@ class TestObjectListBase(test.TestCase):
                 if issubclass(obj_class, base.ObjectListBase):
                     self._test_object_list_version_mappings(obj_class)
 
+    def test_list_changes(self):
+        class Foo(base.ObjectListBase, base.NovaObject):
+            fields = {'objects': fields.ListOfObjectsField('Bar')}
+
+        class Bar(base.NovaObject):
+            fields = {'foo': fields.StringField()}
+
+        obj = Foo(objects=[])
+        self.assertEqual(set(['objects']), obj.obj_what_changed())
+        obj.objects.append(Bar(foo='test'))
+        self.assertEqual(set(['objects']), obj.obj_what_changed())
+        obj.obj_reset_changes()
+        # This should still look dirty because the child is dirty
+        self.assertEqual(set(['objects']), obj.obj_what_changed())
+        obj.objects[0].obj_reset_changes()
+        # This should now look clean because the child is clean
+        self.assertEqual(set(), obj.obj_what_changed())
+
 
 class TestObjectSerializer(_BaseTestCase):
     def test_serialize_entity_primitive(self):