Fix saving tz aware datetimes in Versioned Objects

Currently we cannot save date fields in Versioned Objects because they
are timezone aware and our database is not.

If we look at our objects definition, we are not giving any value to
tzinfo_aware argument, so it takes DateTime default value (True).

In our sqlalchemy model definitions we are defining our datetime fields
as Column(DateTime) which will take sqlalchemy default: class
sqlalchemy.types.DateTime(timezone=False)

In most cases we are accessing those fields directly through the DB
methods, so we don't see the problem, but if we were to modify any of
those fields using versioned object, for example scheduled_at, and then
save the object we'll get an exception:

 File ".../sqlalchemy/sql/type_api.py", line 278, in compare_values
 return x == y
 TypeError: can't compare offset-naive and offset-aware datetimes

Because we are trying to save a timezone aware date to a timezone
unaware DB field.

Following Dan's advice we create a specific cinder_obj_get_changes,
instead of overwriting obj_get_changes, that returns all datefields as
timezone naive UTC datetimes ready for saving.

Change-Id: Ie7b0249e3f6850e7c70d23fc53cfaf27fe04afbb
Closes-Bug: #1469120

cinder/objects/backup.py

@@ -90,14 +90,14 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
         if self.obj_attr_is_set('id'):
             raise exception.ObjectActionError(action='create',
                                               reason='already created')
-        updates = self.obj_get_changes()
+        updates = self.cinder_obj_get_changes()
 
         db_backup = db.backup_create(self._context, updates)
         self._from_db_object(self._context, self, db_backup)
 
     @base.remotable
     def save(self):
-        updates = self.obj_get_changes()
+        updates = self.cinder_obj_get_changes()
         if updates:
             db.backup_update(self._context, self.id, updates)
 

cinder/objects/base.py

@@ -46,6 +46,28 @@ class CinderObject(base.VersionedObject):
     # from one another.
     OBJ_PROJECT_NAMESPACE = 'cinder'
 
+    def cinder_obj_get_changes(self):
+        """Returns a dict of changed fields with tz unaware datetimes.
+
+        Any timezone aware datetime field will be converted to UTC timezone
+        and returned as timezone unaware datetime.
+
+        This will allow us to pass these fields directly to a db update
+        method as they can't have timezone information.
+        """
+        # Get dirtied/changed fields
+        changes = self.obj_get_changes()
+
+        # Look for datetime objects that contain timezone information
+        for k, v in changes.items():
+            if isinstance(v, datetime.datetime) and v.tzinfo:
+                # Remove timezone information and adjust the time according to
+                # the timezone information's offset.
+                changes[k] = v.replace(tzinfo=None) - v.utcoffset()
+
+        # Return modified dict
+        return changes
+
 
 class CinderObjectDictCompat(base.VersionedObjectDictCompat):
     """Mix-in to provide dictionary key access compat.

cinder/objects/snapshot.py

@@ -136,7 +136,7 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject,
         if self.obj_attr_is_set('id'):
             raise exception.ObjectActionError(action='create',
                                               reason=_('already created'))
-        updates = self.obj_get_changes()
+        updates = self.cinder_obj_get_changes()
 
         if 'volume' in updates:
             raise exception.ObjectActionError(action='create',
@@ -147,7 +147,7 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject,
 
     @base.remotable
     def save(self):
-        updates = self.obj_get_changes()
+        updates = self.cinder_obj_get_changes()
         if updates:
             if 'volume' in updates:
                 raise exception.ObjectActionError(action='save',

cinder/objects/volume.py

@@ -121,13 +121,13 @@ class Volume(base.CinderPersistentObject, base.CinderObject,
         if self.obj_attr_is_set('id'):
             raise exception.ObjectActionError(action='create',
                                               reason=_('already created'))
-        updates = self.obj_get_changes()
+        updates = self.cinder_obj_get_changes()
         db_volume = db.volume_create(self._context, updates)
         self._from_db_object(self._context, self, db_volume)
 
     @base.remotable
     def save(self):
-        updates = self.obj_get_changes()
+        updates = self.cinder_obj_get_changes()
         if updates:
             db.volume_update(self._context, self.id, updates)
 

cinder/tests/unit/objects/test_base.py

@@ -0,0 +1,77 @@
+#    Copyright 2015 Red Hat, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import datetime
+import uuid
+
+from iso8601 import iso8601
+
+from cinder.objects import base
+from cinder.tests.unit import objects as test_objects
+
+
+@base.CinderObjectRegistry.register
+class TestObject(base.CinderObject):
+    fields = {
+        'scheduled_at': base.fields.DateTimeField(nullable=True),
+        'uuid': base.fields.UUIDField(),
+        'text': base.fields.StringField(nullable=True),
+    }
+
+
+class TestCinderObject(test_objects.BaseObjectsTestCase):
+    """Tests methods from CinderObject."""
+
+    def setUp(self):
+        super(TestCinderObject, self).setUp()
+        self.obj = TestObject(
+            scheduled_at=None,
+            uuid=uuid.uuid4(),
+            text='text')
+        self.obj.obj_reset_changes()
+
+    def test_cinder_obj_get_changes_no_changes(self):
+        self.assertDictEqual({}, self.obj.cinder_obj_get_changes())
+
+    def test_cinder_obj_get_changes_other_changes(self):
+        self.obj.text = 'text2'
+        self.assertDictEqual({'text': 'text2'},
+                             self.obj.cinder_obj_get_changes())
+
+    def test_cinder_obj_get_changes_datetime_no_tz(self):
+        now = datetime.datetime.utcnow()
+        self.obj.scheduled_at = now
+        self.assertDictEqual({'scheduled_at': now},
+                             self.obj.cinder_obj_get_changes())
+
+    def test_cinder_obj_get_changes_datetime_tz_utc(self):
+        now_tz = iso8601.parse_date('2015-06-26T22:00:01Z')
+        now = now_tz.replace(tzinfo=None)
+        self.obj.scheduled_at = now_tz
+        self.assertDictEqual({'scheduled_at': now},
+                             self.obj.cinder_obj_get_changes())
+
+    def test_cinder_obj_get_changes_datetime_tz_non_utc_positive(self):
+        now_tz = iso8601.parse_date('2015-06-26T22:00:01+01')
+        now = now_tz.replace(tzinfo=None) - datetime.timedelta(hours=1)
+        self.obj.scheduled_at = now_tz
+        self.assertDictEqual({'scheduled_at': now},
+                             self.obj.cinder_obj_get_changes())
+
+    def test_cinder_obj_get_changes_datetime_tz_non_utc_negative(self):
+        now_tz = iso8601.parse_date('2015-06-26T10:00:01-05')
+        now = now_tz.replace(tzinfo=None) + datetime.timedelta(hours=5)
+        self.obj.scheduled_at = now_tz
+        self.assertDictEqual({'scheduled_at': now},
+                             self.obj.cinder_obj_get_changes())