From 990d4c799f2a48530616e289418af395d168b56c Mon Sep 17 00:00:00 2001 From: Diana Clarke Date: Tue, 14 Mar 2017 13:03:11 -0400 Subject: [PATCH] detect and raise for version_id is NULL The versioning feature does not support NULL for the version counter. An exception is now raised if the version id is programmatic and was set to NULL for an UPDATE. Pull request courtesy Diana Clarke. Fixes: #3673 Change-Id: I8b0da56234a7c7f5e7fde35536e09a6216a5e48a --- doc/build/changelog/changelog_12.rst | 7 +++ lib/sqlalchemy/orm/persistence.py | 5 ++ test/orm/test_versioning.py | 73 ++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst index 8cc8cd862b..2d15358490 100644 --- a/doc/build/changelog/changelog_12.rst +++ b/doc/build/changelog/changelog_12.rst @@ -26,6 +26,13 @@ and the UPDATE now sets the version_id value to itself, so that concurrency checks still take place. + .. change:: 3673 + :tags: bug, orm + :tickets: 3673 + + The versioning feature does not support NULL for the version counter. + An exception is now raised if the version id is programmatic and + was set to NULL for an UPDATE. Pull request courtesy Diana Clarke. .. change:: 3796 :tags: bug, orm diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index 86125d6f22..e8a7e4c33e 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -1032,6 +1032,11 @@ def _finalize_insert_update_commands(base_mapper, uowtransaction, states): else: mapper.dispatch.after_update(mapper, connection, state) + if mapper.version_id_col is not None: + if state_dict[mapper._version_id_prop.key] is None: + raise orm_exc.FlushError( + "Instance does not contain a non-NULL version value") + def _postfetch(mapper, uowtransaction, table, state, dict_, result, params, value_params): diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py index 4c339d3bb5..9ade0a4ff1 100644 --- a/test/orm/test_versioning.py +++ b/test/orm/test_versioning.py @@ -20,6 +20,79 @@ def make_uuid(): return uuid.uuid4().hex +class NullVersionIdTest(fixtures.MappedTest): + __backend__ = True + + @classmethod + def define_tables(cls, metadata): + Table('version_table', metadata, + Column('id', Integer, primary_key=True, + test_needs_autoincrement=True), + Column('version_id', Integer), + Column('value', String(40), nullable=False)) + + @classmethod + def setup_classes(cls): + class Foo(cls.Basic): + pass + + def _fixture(self): + Foo, version_table = self.classes.Foo, self.tables.version_table + + mapper( + Foo, version_table, + version_id_col=version_table.c.version_id, + version_id_generator=False, + ) + + s1 = Session() + return s1 + + def test_null_version_id_insert(self): + Foo = self.classes.Foo + + s1 = self._fixture() + f1 = Foo(value='f1') + s1.add(f1) + + # Prior to the fix for #3673, you would have been allowed to insert + # the above record with a NULL version_id and you would have gotten + # the following error when you tried to update it. Now you should + # get a FlushError on the initial insert. + # + # A value is required for bind parameter 'version_table_version_id' + # UPDATE version_table SET value=? + # WHERE version_table.id = ? + # AND version_table.version_id = ? + # parameters: [{'version_table_id': 1, 'value': 'f1rev2'}]] + + assert_raises_message( + sa.orm.exc.FlushError, + "Instance does not contain a non-NULL version value", + s1.commit) + + def test_null_version_id_update(self): + Foo = self.classes.Foo + + s1 = self._fixture() + f1 = Foo(value='f1', version_id=1) + s1.add(f1) + s1.commit() + + # Prior to the fix for #3673, you would have been allowed to update + # the above record with a NULL version_id, and it would look like + # this, post commit: Foo(id=1, value='f1rev2', version_id=None). Now + # you should get a FlushError on update. + + f1.value = 'f1rev2' + f1.version_id = None + + assert_raises_message( + sa.orm.exc.FlushError, + "Instance does not contain a non-NULL version value", + s1.commit) + + class VersioningTest(fixtures.MappedTest): __backend__ = True -- 2.39.5