From: Mike Bayer Date: Tue, 16 Feb 2010 23:33:20 +0000 (+0000) Subject: - Fixed cascade bug in many-to-one relation() when attribute X-Git-Tag: rel_0_6beta2~173 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1f7e0c069c68f60254424081bd184d2955a3b25a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Fixed cascade bug in many-to-one relation() when attribute was set to None, introduced in r6711 (cascade deleted items into session during add()). --- diff --git a/CHANGES b/CHANGES index 4141c3d8e1..e8582ebf65 100644 --- a/CHANGES +++ b/CHANGES @@ -67,7 +67,11 @@ CHANGES - Documentation clarification for query.delete() [ticket:1689] - + + - Fixed cascade bug in many-to-one relation() when attribute + was set to None, introduced in r6711 (cascade deleted + items into session during add()). + - sql - The most common result processors conversion function were moved to the new "processors" module. Dialect authors are diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 46dc6301a3..b0ad3ae94d 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -113,16 +113,22 @@ class DependencyProcessor(object): raise NotImplementedError() def _verify_canload(self, state): - if state is not None and not self.mapper._canload(state, allow_subtypes=not self.enable_typechecks): + if state is not None and \ + not self.mapper._canload(state, allow_subtypes=not self.enable_typechecks): if self.mapper._canload(state, allow_subtypes=True): - raise exc.FlushError("Attempting to flush an item of type %s on collection '%s', " - "which is not the expected type %s. Configure mapper '%s' to load this " - "subtype polymorphically, or set enable_typechecks=False to allow subtypes. " - "Mismatched typeloading may cause bi-directional relationships (backrefs) " - "to not function properly." % (state.class_, self.prop, self.mapper.class_, self.mapper)) + raise exc.FlushError( + "Attempting to flush an item of type %s on collection '%s', " + "which is not the expected type %s. Configure mapper '%s' to " + "load this subtype polymorphically, or set " + "enable_typechecks=False to allow subtypes. " + "Mismatched typeloading may cause bi-directional relationships " + "(backrefs) to not function properly." % + (state.class_, self.prop, self.mapper.class_, self.mapper)) else: - raise exc.FlushError("Attempting to flush an item of type %s on collection '%s', " - "whose mapper does not inherit from that of %s." % (state.class_, self.prop, self.mapper.class_)) + raise exc.FlushError( + "Attempting to flush an item of type %s on collection '%s', " + "whose mapper does not inherit from that of %s." % + (state.class_, self.prop, self.mapper.class_)) def _synchronize(self, state, child, associationrow, clearkeys, uowcommit): """Called during a flush to synchronize primary key identifier diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index bb92f39e49..7c605dd8e2 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -704,13 +704,19 @@ class RelationProperty(StrategizedProperty): if instances: for c in instances: - if c is not None and c not in visited_instances and \ - (halt_on is None or not halt_on(c)): + if c is not None and \ + c is not attributes.PASSIVE_NO_RESULT and \ + c not in visited_instances and \ + (halt_on is None or not halt_on(c)): + if not isinstance(c, self.mapper.class_): - raise AssertionError("Attribute '%s' on class '%s' doesn't handle objects " - "of type '%s'" % (self.key, - str(self.parent.class_), - str(c.__class__))) + raise AssertionError("Attribute '%s' on class '%s' " + "doesn't handle objects " + "of type '%s'" % ( + self.key, + str(self.parent.class_), + str(c.__class__) + )) visited_instances.add(c) # cascade using the mapper local to this diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index d5246bee0a..c7a5a361b9 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -1051,7 +1051,7 @@ class Session(object): def _cascade_save_or_update(self, state): for state, mapper in _cascade_unknown_state_iterator( - 'save-update', state, halt_on=lambda c:c in self): + 'save-update', state, halt_on=self.__contains__): self._save_or_update_impl(state) def delete(self, instance): diff --git a/test/orm/test_cascade.py b/test/orm/test_cascade.py index 7264ed8243..6593d69f68 100644 --- a/test/orm/test_cascade.py +++ b/test/orm/test_cascade.py @@ -75,7 +75,8 @@ class O2MCascadeTest(_fixtures.FixtureTest): assert o2 in sess assert o3 in sess sess.commit() - + + @testing.resolve_artifact_names def test_delete(self): sess = create_session() @@ -375,8 +376,14 @@ class M2OCascadeTest(_base.MappedTest): Table('users', metadata, Column('id', Integer, primary_key=True, test_needs_autoincrement=True), Column('name', String(40)), - Column('pref_id', Integer, ForeignKey('prefs.id'))) + Column('pref_id', Integer, ForeignKey('prefs.id')), + Column('foo_id', Integer, ForeignKey('foo.id')) + ) + Table('foo', metadata, + Column('id', Integer, primary_key=True, test_needs_autoincrement=True), + Column('data', String(40)) + ) @classmethod def setup_classes(cls): class User(_fixtures.Base): @@ -385,7 +392,9 @@ class M2OCascadeTest(_base.MappedTest): pass class Extra(_fixtures.Base): pass - + class Foo(_fixtures.Base): + pass + @classmethod @testing.resolve_artifact_names def setup_mappers(cls): @@ -394,8 +403,10 @@ class M2OCascadeTest(_base.MappedTest): extra = relation(Extra, cascade="all, delete") )) mapper(User, users, properties = dict( - pref = relation(Pref, lazy=False, cascade="all, delete-orphan", single_parent=True ) + pref = relation(Pref, lazy=False, cascade="all, delete-orphan", single_parent=True ), + foo = relation(Foo) # straight m2o )) + mapper(Foo, foo) @classmethod @testing.resolve_artifact_names @@ -420,6 +431,31 @@ class M2OCascadeTest(_base.MappedTest): assert prefs.count().scalar() == 2 assert extra.count().scalar() == 2 + @testing.resolve_artifact_names + def test_cascade_on_deleted(self): + """test a bug introduced by r6711""" + + sess = sessionmaker(expire_on_commit=True)() + + + u1 = User(name='jack', foo=Foo(data='f1')) + sess.add(u1) + sess.commit() + + u1.foo = None + + # the error condition relies upon + # these things being true + assert User.foo.impl.active_history is False + eq_( + attributes.get_history(u1, 'foo'), + ([None], (), [attributes.PASSIVE_NO_RESULT]) + ) + + sess.add(u1) + assert u1 in sess + sess.commit() + @testing.resolve_artifact_names def test_save_update_sends_pending(self): """test that newly added and deleted scalar items are cascaded on save-update"""