From cf33a85efb3a3adddc60d05424012847123bfd95 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Fri, 26 Jun 2015 16:05:04 +0200 Subject: [PATCH] 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 | 4 +- cinder/objects/base.py | 22 ++++++++ cinder/objects/snapshot.py | 4 +- cinder/objects/volume.py | 4 +- cinder/tests/unit/objects/test_base.py | 77 ++++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 cinder/tests/unit/objects/test_base.py diff --git a/cinder/objects/backup.py b/cinder/objects/backup.py index 760a633fc..3fdc0b04b 100644 --- a/cinder/objects/backup.py +++ b/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) diff --git a/cinder/objects/base.py b/cinder/objects/base.py index 7b687190c..240cb6179 100644 --- a/cinder/objects/base.py +++ b/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. diff --git a/cinder/objects/snapshot.py b/cinder/objects/snapshot.py index c9d84f147..3e69d3eda 100644 --- a/cinder/objects/snapshot.py +++ b/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', diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index d32aff26a..1dab3a234 100644 --- a/cinder/objects/volume.py +++ b/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) diff --git a/cinder/tests/unit/objects/test_base.py b/cinder/tests/unit/objects/test_base.py new file mode 100644 index 000000000..aa0b83c6d --- /dev/null +++ b/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()) -- 2.45.2