- 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 4141c3d8e1926bdb0d2a3027054f3de47393c8d6..e8582ebf653c684c61b971c03bb8c94d56b20480 100644 (file)
--- 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 46dc6301a39cf10833f22ea9d467aa10ce491a32..b0ad3ae94dccdf1d91ef806a5c85fd5c5c7e43b9 100644 (file)
--- 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 bb92f39e497e435864abee60c74e969fc10f6dd8..7c605dd8e266b84a70a4e01878015e52dd05ed65 100644 (file)
--- 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 d5246bee0ab66cb4197d00facae4f10878e96724..c7a5a361b97ebbcb02b28ea10d08b233a400ab24 100644 (file)
--- 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 7264ed82439b2f87c2c7db46272ef2e15c9dcfd0..6593d69f681aae523598c2cf7be97c08822827a9 100644 (file)
--- 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"""