diff --git a/CHANGES b/CHANGES index 9cc4e1c15d..8b555f5a83 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,15 @@ CHANGES 0.6.4 ===== - orm + - The name ConcurrentModificationError has been + changed to StaleDataError, and descriptive + error messages have been revised to reflect + exactly what the issue is. Both names will + remain available for the forseeable future + for schemes that may be specifying + ConcurrentModificationError in an "except:" + clause. + - Moving an o2m object from one collection to another, or vice versa changing the referenced object by an m2o, where the foreign key is also a diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index 1252baa071..4c9efb714b 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -782,7 +782,7 @@ def mapper(class_, local_table=None, *args, **params): a running *version id* of mapped entities in the database. this is used during save operations to ensure that no other thread or process has updated the instance during the lifetime of the entity, else a - ``ConcurrentModificationError`` exception is thrown. + :class:`StaleDataError` exception is thrown. :param version_id_generator: A callable which defines the algorithm used to generate new version ids. Defaults to an integer generator. Can be replaced with one that diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index e96eed28ea..376afd88d4 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -1032,14 +1032,12 @@ class ManyToManyDP(DependencyProcessor): if result.supports_sane_multi_rowcount() and \ result.rowcount != len(secondary_delete): - raise exc.ConcurrentModificationError( - "Deleted rowcount %d does not match number of " - "secondary table rows deleted from table '%s': %d" % - ( - result.rowcount, - self.secondary.description, - len(secondary_delete)) - ) + raise exc.StaleDataError( + "DELETE statement on table '%s' expected to delete %d row(s); " + "Only %d were matched." % + (self.secondary.description, len(secondary_delete), + result.rowcount) + ) if secondary_update: associationrow = secondary_update[0] @@ -1051,14 +1049,12 @@ class ManyToManyDP(DependencyProcessor): result = connection.execute(statement, secondary_update) if result.supports_sane_multi_rowcount() and \ result.rowcount != len(secondary_update): - raise exc.ConcurrentModificationError( - "Updated rowcount %d does not match number of " - "secondary table rows updated from table '%s': %d" % - ( - result.rowcount, - self.secondary.description, - len(secondary_update)) - ) + raise exc.StaleDataError( + "UPDATE statement on table '%s' expected to update %d row(s); " + "Only %d were matched." % + (self.secondary.description, len(secondary_update), + result.rowcount) + ) if secondary_insert: statement = self.secondary.insert() diff --git a/lib/sqlalchemy/orm/exc.py b/lib/sqlalchemy/orm/exc.py index 431acc15cc..3f28a3dd32 100644 --- a/lib/sqlalchemy/orm/exc.py +++ b/lib/sqlalchemy/orm/exc.py @@ -12,8 +12,25 @@ import sqlalchemy as sa NO_STATE = (AttributeError, KeyError) """Exception types that may be raised by instrumentation implementations.""" -class ConcurrentModificationError(sa.exc.SQLAlchemyError): - """Rows have been modified outside of the unit of work.""" +class StaleDataError(sa.exc.SQLAlchemyError): + """An operation encountered database state that is unaccounted for. + + Two conditions cause this to happen: + + * A flush may have attempted to update or delete rows + and an unexpected number of rows were matched during + the UPDATE or DELETE statement. Note that when + version_id_col is used, rows in UPDATE or DELETE statements + are also matched against the current known version + identifier. + + * A mapped object with version_id_col was refreshed, + and the version number coming back from the database does + not match that of the object itself. + + """ + +ConcurrentModificationError = StaleDataError class FlushError(sa.exc.SQLAlchemyError): @@ -24,7 +41,8 @@ class UnmappedError(sa.exc.InvalidRequestError): """TODO""" class DetachedInstanceError(sa.exc.SQLAlchemyError): - """An attempt to access unloaded attributes on a mapped instance that is detached.""" + """An attempt to access unloaded attributes on a + mapped instance that is detached.""" class UnmappedInstanceError(UnmappedError): """An mapping operation was requested for an unknown instance.""" diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 6c25b89caf..9cb3581517 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1818,10 +1818,10 @@ class Mapper(object): if connection.dialect.supports_sane_rowcount: if rows != len(update): - raise orm_exc.ConcurrentModificationError( - "Updated rowcount %d does not match number " - "of objects updated %d" % - (rows, len(update))) + raise orm_exc.StaleDataError( + "UPDATE statement on table '%s' expected to update %d row(s); " + "%d were matched." % + (table.description, len(update), rows)) elif needs_version_id: util.warn("Dialect %s does not support updated rowcount " @@ -2050,10 +2050,10 @@ class Mapper(object): rows = c.rowcount if rows != -1 and rows != len(del_objects): - raise orm_exc.ConcurrentModificationError( - "Deleted rowcount %d does not match " - "number of objects deleted %d" % - (c.rowcount, len(del_objects)) + raise orm_exc.StaleDataError( + "DELETE statement on table '%s' expected to delete %d row(s); " + "%d were matched." % + (table.description, len(del_objects), c.rowcount) ) for state, state_dict, mapper, has_identity, connection in tups: @@ -2180,8 +2180,9 @@ class Mapper(object): self.version_id_col) != \ row[version_id_col]: - raise orm_exc.ConcurrentModificationError( - "Instance '%s' version of %s does not match %s" + raise orm_exc.StaleDataError( + "Instance '%s' has version id '%s' which " + "does not match database-loaded version id '%s'." % (state_str(state), self._get_state_attr_by_column( state, dict_, diff --git a/lib/sqlalchemy/test/requires.py b/lib/sqlalchemy/test/requires.py index 1b9052fd8b..fefb00330a 100644 --- a/lib/sqlalchemy/test/requires.py +++ b/lib/sqlalchemy/test/requires.py @@ -247,6 +247,12 @@ def sane_rowcount(fn): skip_if(lambda: not testing.db.dialect.supports_sane_rowcount) ) +def sane_multi_rowcount(fn): + return _chain_decorators_on( + fn, + skip_if(lambda: not testing.db.dialect.supports_sane_multi_rowcount) + ) + def reflects_pk_names(fn): """Target driver reflects the name of primary key constraints.""" return _chain_decorators_on( diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index 2f9295e17e..7607bd0829 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -605,14 +605,14 @@ class VersioningTest(_base.MappedTest): sess.flush() - assert_raises(orm_exc.ConcurrentModificationError, + assert_raises(orm_exc.StaleDataError, sess2.query(Base).with_lockmode('read').get, s1.id) if not testing.db.dialect.supports_sane_rowcount: sess2.flush() else: - assert_raises(orm_exc.ConcurrentModificationError, sess2.flush) + assert_raises(orm_exc.StaleDataError, sess2.flush) sess2.refresh(s2) if testing.db.dialect.supports_sane_rowcount: @@ -652,12 +652,14 @@ class VersioningTest(_base.MappedTest): s2.subdata = 'some new subdata' sess.flush() - try: - s1.subdata = 'some new subdata' + s1.subdata = 'some new subdata' + if testing.db.dialect.supports_sane_rowcount: + assert_raises( + orm_exc.StaleDataError, + sess.flush + ) + else: sess.flush() - assert not testing.db.dialect.supports_sane_rowcount - except orm_exc.ConcurrentModificationError, e: - assert True class DistinctPKTest(_base.MappedTest): """test the construction of mapper.primary_key when an inheriting relationship diff --git a/test/orm/test_manytomany.py b/test/orm/test_manytomany.py index 46d0cc44fd..f8e1caa8e9 100644 --- a/test/orm/test_manytomany.py +++ b/test/orm/test_manytomany.py @@ -1,10 +1,12 @@ -from sqlalchemy.test.testing import assert_raises, assert_raises_message, eq_ +from sqlalchemy.test.testing import assert_raises, \ + assert_raises_message, eq_ import sqlalchemy as sa from sqlalchemy.test import testing from sqlalchemy import Integer, String, ForeignKey from sqlalchemy.test.schema import Table from sqlalchemy.test.schema import Column -from sqlalchemy.orm import mapper, relationship, create_session +from sqlalchemy.orm import mapper, relationship, create_session, \ + exc as orm_exc, sessionmaker from test.orm import _base @@ -219,7 +221,47 @@ class M2MTest(_base.MappedTest): self.assert_result([t1], Transition, {'outputs': (Place, [{'name':'place3'}, {'name':'place1'}])}) self.assert_result([p2], Place, {'inputs': (Transition, [{'name':'transition1'},{'name':'transition2'}])}) + @testing.requires.sane_multi_rowcount + @testing.resolve_artifact_names + def test_stale_conditions(self): + mapper(Place, place, properties={ + 'transitions':relationship(Transition, secondary=place_input, + passive_updates=False) + }) + mapper(Transition, transition) + + p1 = Place('place1') + t1 = Transition('t1') + p1.transitions.append(t1) + sess = sessionmaker()() + sess.add_all([p1, t1]) + sess.commit() + p1.place_id + p1.transitions + + sess.execute("delete from place_input", mapper=Place) + p1.place_id = 7 + + assert_raises_message( + orm_exc.StaleDataError, + r"UPDATE statement on table 'place_input' expected to " + r"update 1 row\(s\); Only 0 were matched.", + sess.commit + ) + sess.rollback() + + p1.place_id + p1.transitions + sess.execute("delete from place_input", mapper=Place) + p1.transitions.remove(t1) + assert_raises_message( + orm_exc.StaleDataError, + r"DELETE statement on table 'place_input' expected to " + r"delete 1 row\(s\); Only 0 were matched.", + sess.commit + ) + class M2MTest2(_base.MappedTest): @classmethod def define_tables(cls, metadata): diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py index 783c30ab32..6cb2bb9e2c 100644 --- a/test/orm/test_versioning.py +++ b/test/orm/test_versioning.py @@ -8,11 +8,22 @@ from test.orm import _base, _fixtures from test.engine import _base as engine_base -_uuids =['1fc614acbb904742a2990f86af6ded95', '23e253786f4d491b9f9d6189dc33de9b', 'fc44910db37e43fd93e9ec8165b885cf', - '0187a1832b4249e6b48911821d86de58', '778af6ea2fb74a009d8d2f5abe5dc29a', '51a6ce031aff47e4b5f2895c4161f120', - '7434097cd319401fb9f15fa443ccbbbb', '9bc548a8128e4a85ac18060bc3f4b7d3', '59548715e3c440b7bcb96417d06f7930', - 'd7647c7004734de196885ca2bd73adf8', '70cef121d3ff48d39906b6d1ac77f41a', 'ee37a8a6430c466aa322b8a215a0dd70', - '782a5f04b4364a53a6fce762f48921c1', 'bef510f2420f4476a7629013ead237f5'] +_uuids = [ + '1fc614acbb904742a2990f86af6ded95', + '23e253786f4d491b9f9d6189dc33de9b', + 'fc44910db37e43fd93e9ec8165b885cf', + '0187a1832b4249e6b48911821d86de58', + '778af6ea2fb74a009d8d2f5abe5dc29a', + '51a6ce031aff47e4b5f2895c4161f120', + '7434097cd319401fb9f15fa443ccbbbb', + '9bc548a8128e4a85ac18060bc3f4b7d3', + '59548715e3c440b7bcb96417d06f7930', + 'd7647c7004734de196885ca2bd73adf8', + '70cef121d3ff48d39906b6d1ac77f41a', + 'ee37a8a6430c466aa322b8a215a0dd70', + '782a5f04b4364a53a6fce762f48921c1', + 'bef510f2420f4476a7629013ead237f5', + ] def make_uuid(): """generate uuids even on Python 2.4 which has no 'uuid'""" @@ -85,9 +96,12 @@ class VersioningTest(_base.MappedTest): f1.value='f1rev3mine' # Only dialects with a sane rowcount can detect the - # ConcurrentModificationError + # StaleDataError if testing.db.dialect.supports_sane_rowcount: - assert_raises(sa.orm.exc.ConcurrentModificationError, s1.commit) + assert_raises_message(sa.orm.exc.StaleDataError, + r"UPDATE statement on table 'version_table' expected " + r"to update 1 row\(s\); 0 were matched.", + s1.commit), s1.rollback() else: s1.commit() @@ -103,7 +117,11 @@ class VersioningTest(_base.MappedTest): s1.delete(f2) if testing.db.dialect.supports_sane_rowcount: - assert_raises(sa.orm.exc.ConcurrentModificationError, s1.commit) + assert_raises_message( + sa.orm.exc.StaleDataError, + r"DELETE statement on table 'version_table' expected " + r"to delete 2 row\(s\); 1 were matched.", + s1.commit) else: s1.commit() @@ -160,8 +178,10 @@ class VersioningTest(_base.MappedTest): s2.commit() # load, version is wrong - assert_raises( - sa.orm.exc.ConcurrentModificationError, + assert_raises_message( + sa.orm.exc.StaleDataError, + r"Instance .* has version id '\d+' which does not " + r"match database-loaded version id '\d+'", s1.query(Foo).with_lockmode('read').get, f1s1.id ) @@ -369,8 +389,10 @@ class AlternateGeneratorTest(_base.MappedTest): sess1.commit() p2.data = 'P overwritten by concurrent tx' - assert_raises( - orm.exc.ConcurrentModificationError, + assert_raises_message( + orm.exc.StaleDataError, + r"UPDATE statement on table 'p' expected to update " + r"1 row\(s\); 0 were matched.", sess2.commit )