Fix CinderPersistentObject.refresh

Currently our refresh method is not working as it should as it has
mainly 3 problems:

- It doesn't support read_only fields
- It may trigger lazy loads
- It will not respect version-fields relationships

The read_only issue is triggered when we do a refresh from the DB and
the field has changed.  This could be the case of a metadata field that
is loaded by `get_by_id` but it is not allowed to be modified directly
in the OVO:

    oslo_versionedobjects.exception.ReadOnlyFieldError: Cannot modify
    readonly field name

The triggering of lazy loads would occur if we have an OVO that has an
optional field set and we do a refresh.  This refresh would trigger the
lazy load of that field.

A case where the old refresh would not respect version-fields
relationships is for example when you receive in a node with volume v1.1
and CG v1.1 a volume v1.0 that has CG v1.0, then you do a refresh and
you end up with a volume with v1.0 and CG v1.1.  And this gets even
worse if any of your fields changed the stored contents on a field and
relied on backporting to do the right mapping, as now we'll have the old
OVO version with the new OVO contents.

This patch refactors refresh method in a way that fixes the first 2
problems as it has been deemed that proposed solution for the third
issue would add too much complexity and it's more likely unnecessary.

Closes-Bug: #1599502
Change-Id: Ibcb0f841d92ab7f96c076fba4569c56734e91ffd

cinder/objects/base.py

@@ -410,14 +410,10 @@ class CinderPersistentObject(object):
 
         current = self.get_by_id(self._context, self.id)
 
-        for field in self.fields:
-            # Only update attributes that are already set.  We do not want to
-            # unexpectedly trigger a lazy-load.
-            if self.obj_attr_is_set(field):
-                current_field = getattr(current, field)
-                if getattr(self, field) != current_field:
-                    setattr(self, field, current_field)
-        self.obj_reset_changes()
+        # Copy contents retrieved from the DB into self
+        my_data = vars(self)
+        my_data.clear()
+        my_data.update(vars(current))
 
     @classmethod
     def exists(cls, context, id_):

cinder/tests/unit/objects/test_base.py

@@ -94,6 +94,23 @@ class TestCinderObject(test_objects.BaseObjectsTestCase):
         test_obj.refresh()
         self._compare(self, refresh_obj, test_obj)
 
+    @mock.patch('cinder.objects.base.CinderPersistentObject.get_by_id')
+    def test_refresh_readonly(self, get_by_id_mock):
+        @objects.base.CinderObjectRegistry.register_if(False)
+        class MyTestObject(objects.base.CinderObject,
+                           objects.base.CinderObjectDictCompat,
+                           objects.base.CinderComparableObject,
+                           objects.base.CinderPersistentObject):
+            fields = {'id': fields.UUIDField(),
+                      'name': fields.StringField(read_only=True)}
+
+        test_obj = MyTestObject(id=fake.OBJECT_ID, name='foo')
+        refresh_obj = MyTestObject(id=fake.OBJECT_ID, name='bar')
+        get_by_id_mock.return_value = refresh_obj
+
+        test_obj.refresh()
+        self._compare(self, refresh_obj, test_obj)
+
     def test_refresh_no_id_field(self):
         @objects.base.CinderObjectRegistry.register_if(False)
         class MyTestObjectNoId(objects.base.CinderObject,