From: Mike Bayer Date: Sun, 1 Aug 2010 22:24:35 +0000 (-0400) Subject: - The name ConcurrentModificationError has been X-Git-Tag: rel_0_6_4~69 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c09b79d61eaba130efcd676db5e27ac3635535d2;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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. --- 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 )