diff options
author | Roman Podoliaka <rpodolyaka@mirantis.com> | 2016-11-28 13:16:17 +0200 |
---|---|---|
committer | Roman Podoliaka <rpodolyaka@mirantis.com> | 2016-11-28 13:20:50 +0200 |
commit | 8be136528e99d8ea6ae8f1ae8767738b4c876759 (patch) | |
tree | 844548cf3c07dec9530dfc7041dd9c691fecbf5d | |
parent | 34f9a3ac7a56883f8a2cd2a9a93bc42e5194bc1e (diff) | |
download | oslo-db-8be136528e99d8ea6ae8f1ae8767738b4c876759.tar.gz |
SoftDeleteMixin: coerce deleted param to be an integer
PostgreSQL is very strict about types and won't perform an automatic
type cast when trying to pass e.g. a boolean `false` value to be
INSERT'ed or UPDATE'ed.
Coerce the value of deleted bound parameter to always be an integer
in the DB layer by adding a custom SQLAlchemy integer type decorator
to make PostgreSQL happy, should someone pass a boolean value for
deleted column (it was meant to be of boolean type, but is integer
instead to allow for Nova use case of shadow tables and compound
unique constraints on something + deleted).
Closes-Bug: #1644513
Change-Id: I13c6233cfeb611b1b106eedfc9b57d2af313c46b
-rw-r--r-- | oslo_db/sqlalchemy/models.py | 6 | ||||
-rw-r--r-- | oslo_db/sqlalchemy/types.py | 19 | ||||
-rw-r--r-- | oslo_db/tests/sqlalchemy/test_models.py | 50 |
3 files changed, 72 insertions, 3 deletions
diff --git a/oslo_db/sqlalchemy/models.py b/oslo_db/sqlalchemy/models.py index 0c4e0de..0467971 100644 --- a/oslo_db/sqlalchemy/models.py +++ b/oslo_db/sqlalchemy/models.py @@ -23,10 +23,12 @@ SQLAlchemy models. import six from oslo_utils import timeutils -from sqlalchemy import Column, Integer +from sqlalchemy import Column from sqlalchemy import DateTime from sqlalchemy.orm import object_mapper +from oslo_db.sqlalchemy import types + class ModelBase(six.Iterator): """Base class for models.""" @@ -139,7 +141,7 @@ class TimestampMixin(object): class SoftDeleteMixin(object): deleted_at = Column(DateTime) - deleted = Column(Integer, default=0) + deleted = Column(types.SoftDeleteInteger, default=0) def soft_delete(self, session): """Mark this object as deleted.""" diff --git a/oslo_db/sqlalchemy/types.py b/oslo_db/sqlalchemy/types.py index a6f8acb..f74f909 100644 --- a/oslo_db/sqlalchemy/types.py +++ b/oslo_db/sqlalchemy/types.py @@ -12,7 +12,7 @@ import json -from sqlalchemy.types import TypeDecorator, Text +from sqlalchemy.types import Integer, TypeDecorator, Text from sqlalchemy.dialects import mysql @@ -73,3 +73,20 @@ class JsonEncodedList(JsonEncodedType): http://docs.sqlalchemy.org/en/rel_1_0/orm/extensions/mutable.html """ type = list + + +class SoftDeleteInteger(TypeDecorator): + """Coerce a bound param to be a proper integer before passing it to DBAPI. + + Some backends like PostgreSQL are very strict about types and do not + perform automatic type casts, e.g. when trying to INSERT a boolean value + like ``false`` into an integer column. Coercing of the bound param in DB + layer by the means of a custom SQLAlchemy type decorator makes sure we + always pass a proper integer value to a DBAPI implementation. + + """ + + impl = Integer + + def process_bind_param(self, value, dialect): + return int(value) diff --git a/oslo_db/tests/sqlalchemy/test_models.py b/oslo_db/tests/sqlalchemy/test_models.py index 60e8c55..fc12b0b 100644 --- a/oslo_db/tests/sqlalchemy/test_models.py +++ b/oslo_db/tests/sqlalchemy/test_models.py @@ -14,10 +14,13 @@ # under the License. import collections +import datetime +import mock from oslotest import base as oslo_test from sqlalchemy import Column from sqlalchemy import Integer, String +from sqlalchemy import event from sqlalchemy.ext.declarative import declarative_base from oslo_db.sqlalchemy import models @@ -179,3 +182,50 @@ class TimestampMixinTest(oslo_test.BaseTestCase): for method in methods: self.assertTrue(hasattr(models.TimestampMixin, method), "Method %s() is not found" % method) + + +class SoftDeletedModel(BASE, models.ModelBase, models.SoftDeleteMixin): + __tablename__ = 'test_model_soft_deletes' + + id = Column('id', Integer, primary_key=True) + smth = Column('smth', String(255)) + + +class SoftDeleteMixinTest(test_base.DbTestCase): + def setUp(self): + super(SoftDeleteMixinTest, self).setUp() + + t = BASE.metadata.tables['test_model_soft_deletes'] + t.create(self.engine) + self.addCleanup(t.drop, self.engine) + + self.session = self.sessionmaker(autocommit=False) + self.addCleanup(self.session.close) + + @mock.patch('oslo_utils.timeutils.utcnow') + def test_soft_delete(self, mock_utcnow): + dt = datetime.datetime.utcnow().replace(microsecond=0) + mock_utcnow.return_value = dt + + m = SoftDeletedModel(id=123456, smth='test') + self.session.add(m) + self.session.commit() + self.assertEqual(0, m.deleted) + self.assertIs(None, m.deleted_at) + + m.soft_delete(self.session) + self.assertEqual(123456, m.deleted) + self.assertIs(dt, m.deleted_at) + + def test_soft_delete_coerce_deleted_to_integer(self): + def listener(conn, cur, stmt, params, context, executemany): + if 'insert' in stmt.lower(): # ignore SELECT 1 and BEGIN + self.assertNotIn('False', str(params)) + + event.listen(self.engine, 'before_cursor_execute', listener) + self.addCleanup(event.remove, + self.engine, 'before_cursor_execute', listener) + + m = SoftDeletedModel(id=1, smth='test', deleted=False) + self.session.add(m) + self.session.commit() |