- fixed/added coverage for various cascade scenarios

- added coverage for some extra cases in dynamic relations
- removed some unused methods from unitofwork

diff --git a/CHANGES b/CHANGES
index be2e1eb10d0b10e572030014ebc9b0a54f7c782c..3fb68b1ef11a038cc2e3213134b20de92a410940 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -20,10 +20,22 @@ CHANGES
 
     - Session.execute can now find binds from metadata
 
-    - Fixed "cascade delete" operation of dynamic relations, which
-      had only been implemented for foreign-key nulling behavior
-      in 0.4.2 and not actual cascading deletes [ticket:895]
-
+    - assorted "cascade deletes" fixes:
+        - Fixed "cascade delete" operation of dynamic
+          relations, which had only been implemented for
+          foreign-key nulling behavior in 0.4.2 and not actual
+          cascading deletes [ticket:895]
+    
+        - delete cascade without delete-orphan cascade on a
+          many-to-one will not delete orphans which were
+          disconnected from the parent before session.delete()
+          is called on the parent (one-to-many already had
+          this).
+      
+        - delete cascade with delete-orphan will delete
+          orphans whether or not it remains attached to its
+          also-deleted parent.
+    
     - fixed order_by calculation in Query to properly alias
       mapper-config'ed order_by when using select_from()
 

diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index 8b41d93dd9fd8087e7d3a2bb1178601e1a9ef89b..425b171df9e92c22194e485f8f815c5afee64da0 100644 (file)
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -217,16 +217,21 @@ class OneToManyDP(DependencyProcessor):
         if delete:
             # head object is being deleted, and we manage its list of child objects
             # the child objects have to have their foreign key to the parent set to NULL
-            if not self.post_update and not self.cascade.delete and not self.passive_deletes=='all':
+            if not self.post_update:
+                should_null_fks = not self.cascade.delete and not self.passive_deletes=='all'
                 for state in deplist:
                     (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key,passive=self.passive_deletes)
                     if unchanged or deleted:
                         for child in deleted:
                             if child is not None and self.hasparent(child) is False:
-                                uowcommit.register_object(child)
-                        for child in unchanged:
-                            if child is not None:
-                                uowcommit.register_object(child)
+                                if self.cascade.delete_orphan:
+                                    uowcommit.register_object(child, isdelete=True)
+                                else:
+                                    uowcommit.register_object(child)
+                        if should_null_fks:
+                            for child in unchanged:
+                                if child is not None:
+                                    uowcommit.register_object(child)
         else:
             for state in deplist:
                 (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key,passive=True)
@@ -249,8 +254,6 @@ class OneToManyDP(DependencyProcessor):
                             uowcommit.register_object(child)
 
     def _synchronize(self, state, child, associationrow, clearkeys, uowcommit):
-        if child is not None:
-            child = getattr(child, '_state', child)
         source = state
         dest = child
         if dest is None or (not self.post_update and uowcommit.is_deleted(dest)):
@@ -338,15 +341,19 @@ class ManyToOneDP(DependencyProcessor):
         if self.post_update:
             return
         if delete:
-            if self.cascade.delete:
+            if self.cascade.delete or self.cascade.delete_orphan:
                 for state in deplist:
                     (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key,passive=self.passive_deletes)
-                    if deleted or unchanged:
-                        for child in deleted + unchanged:
-                            if child is not None and self.hasparent(child) is False:
-                                uowcommit.register_object(child, isdelete=True)
-                                for c, m in self.mapper.cascade_iterator('delete', child):
-                                    uowcommit.register_object(c._state, isdelete=True)
+                    if self.cascade.delete_orphan:
+                        todelete = added + unchanged + deleted
+                    else:
+                        todelete = added + unchanged
+                    for child in todelete:
+                        if child is None:
+                            continue
+                        uowcommit.register_object(child, isdelete=True)
+                        for c, m in self.mapper.cascade_iterator('delete', child):
+                            uowcommit.register_object(c._state, isdelete=True)
         else:
             for state in deplist:
                 uowcommit.register_object(state)

diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index ae55b4c94324a8328406eb497fcd30b07d8c779a..1676eeb22cd83aa26a0d7b299f09833048a1e140 100644 (file)
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -349,16 +349,6 @@ class UOWTransaction(object):
         taskelement = task._objects[state]
         taskelement.isdelete = "rowswitch"
         
-    def unregister_object(self, obj):
-        """remove an object from its parent UOWTask.
-        
-        called by mapper._save_obj() when an 'identity switch' is detected, so that
-        no further operations occur upon the instance."""
-        mapper = object_mapper(obj)
-        task = self.get_task_by_mapper(mapper)
-        if obj._state in task._objects:
-            task.delete(obj._state)
-
     def is_deleted(self, state):
         """return true if the given state is marked as deleted within this UOWTransaction."""
         
@@ -591,11 +581,6 @@ class UOWTask(object):
         # instead of __eq__
         self.mapper._save_obj([state], self.uowtransaction, postupdate=True, post_update_cols=util.Set(post_update_cols))
 
-    def delete(self, obj):
-        """remove the given object from this UOWTask, if present."""
-
-        self._objects.pop(obj._state, None)
-
     def __contains__(self, state):
         """return True if the given object is contained within this UOWTask or inheriting tasks."""
         

diff --git a/test/orm/cascade.py b/test/orm/cascade.py
index 1abf01de77eeb57d9aa8b67ca86ccc4dd80a96ed..7a68a4d58a994d0e3b83595761fe4601ed110ad7 100644 (file)
--- a/test/orm/cascade.py
+++ b/test/orm/cascade.py
@@ -101,6 +101,20 @@ class O2MCascadeTest(fixtures.FixtureTest):
         assert users.count().scalar() == 1
         assert orders.count().scalar() == 1
         self.assertEquals(sess.query(User).all(), [User(name='newuser', orders=[Order(description='someorder')])])
+
+    def test_cascade_delete_plusorphans(self):
+        sess = create_session()
+        u = User(name='jack', orders=[Order(description='someorder'), Order(description='someotherorder')])
+        sess.save(u)
+        sess.flush()
+        assert users.count().scalar() == 1
+        assert orders.count().scalar() == 2
+
+        del u.orders[0]
+        sess.delete(u)
+        sess.flush()
+        assert users.count().scalar() == 0
+        assert orders.count().scalar() == 0
             
     def test_collection_orphans(self):
         sess = create_session()
@@ -118,6 +132,34 @@ class O2MCascadeTest(fixtures.FixtureTest):
         assert users.count().scalar() == 1
         assert orders.count().scalar() == 0
 
+class O2MCascadeNoOrphanTest(fixtures.FixtureTest):
+    keep_mappers = True
+    keep_data = False
+    refresh_data = False
+
+    def setup_mappers(self):
+        global User, Address, Order, users, orders, addresses
+        from testlib.fixtures import User, Address, Order, users, orders, addresses
+                
+        mapper(User, users, properties = dict(
+            orders = relation(
+                mapper(Order, orders), cascade="all")
+        ))
+    
+    def test_cascade_delete_noorphans(self):
+        sess = create_session()
+        u = User(name='jack', orders=[Order(description='someorder'), Order(description='someotherorder')])
+        sess.save(u)
+        sess.flush()
+        assert users.count().scalar() == 1
+        assert orders.count().scalar() == 2
+        
+        del u.orders[0]
+        sess.delete(u)
+        sess.flush()
+        assert users.count().scalar() == 0
+        assert orders.count().scalar() == 1
+        
 
 class M2OCascadeTest(ORMTest):
     keep_mappers = True
@@ -221,9 +263,192 @@ class M2OCascadeTest(ORMTest):
         sess.flush()
         self.assertEquals(sess.query(Pref).all(), [Pref(data="pref 1"), Pref(data="pref 3"), Pref(data="newpref")])
 
+class M2OCascadeDeleteTest(ORMTest):
+    keep_mappers = True
+    
+    def define_tables(self, metadata):
+        global t1, t2, t3
+        t1 = Table('t1', metadata, Column('id', Integer, primary_key=True), Column('data', String(50)), Column('t2id', Integer, ForeignKey('t2.id')))
+        t2 = Table('t2', metadata, Column('id', Integer, primary_key=True), Column('data', String(50)), Column('t3id', Integer, ForeignKey('t3.id')))
+        t3 = Table('t3', metadata, Column('id', Integer, primary_key=True), Column('data', String(50)))
+    
+    def setup_mappers(self):
+        global T1, T2, T3
+        class T1(fixtures.Base):pass
+        class T2(fixtures.Base):pass
+        class T3(fixtures.Base):pass
+        
+        mapper(T1, t1, properties={'t2':relation(T2, cascade="all")})
+        mapper(T2, t2, properties={'t3':relation(T3, cascade="all")})
+        mapper(T3, t3)
+
+    def test_cascade_delete(self):
+        sess = create_session()
+
+        x = T1(data='t1a', t2=T2(data='t2a', t3=T3(data='t3a')))
+        sess.save(x)
+        sess.flush()
+        
+        sess.delete(x)
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [])
+        self.assertEquals(sess.query(T3).all(), [])
+
+    def test_cascade_delete_postappend_onelevel(self):
+        sess = create_session()
+
+        x1 = T1(data='t1', )
+        x2 = T2(data='t2')
+        x3 = T3(data='t3')
+        sess.save(x1)
+        sess.save(x2)
+        sess.save(x3)
+        sess.flush()
+        
+        sess.delete(x1)
+        x1.t2 = x2
+        x2.t3 = x3
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [])
+        self.assertEquals(sess.query(T3).all(), [])
+
+    def test_cascade_delete_postappend_twolevel(self):
+        sess = create_session()
+
+        x1 = T1(data='t1', t2=T2(data='t2'))
+        x3 = T3(data='t3')
+        sess.save(x1)
+        sess.save(x3)
+        sess.flush()
+
+        sess.delete(x1)
+        x1.t2.t3 = x3
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [])
+        self.assertEquals(sess.query(T3).all(), [])
+        
+    def test_preserves_orphans_onelevel(self):
+        sess = create_session()
+
+        x2 = T1(data='t1b', t2=T2(data='t2b', t3=T3(data='t3b')))
+        sess.save(x2)
+        sess.flush()
+        x2.t2 = None
+
+        sess.delete(x2)
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [T2()])
+        self.assertEquals(sess.query(T3).all(), [T3()])
+
+    @testing.future
+    def test_preserves_orphans_onelevel_postremove(self):
+        sess = create_session()
+
+        x2 = T1(data='t1b', t2=T2(data='t2b', t3=T3(data='t3b')))
+        sess.save(x2)
+        sess.flush()
+
+        sess.delete(x2)
+        x2.t2 = None
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [T2()])
+        self.assertEquals(sess.query(T3).all(), [T3()])
+
+    def test_preserves_orphans_twolevel(self):
+        sess = create_session()
+
+        x = T1(data='t1a', t2=T2(data='t2a', t3=T3(data='t3a')))
+        sess.save(x)
+        sess.flush()
+        
+        x.t2.t3 = None
+        sess.delete(x)
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [])
+        self.assertEquals(sess.query(T3).all(), [T3()])
+
+class M2OCascadeDeleteOrphanTest(ORMTest):
+    keep_mappers = True
+
+    def define_tables(self, metadata):
+        global t1, t2, t3
+        t1 = Table('t1', metadata, Column('id', Integer, primary_key=True), Column('data', String(50)), Column('t2id', Integer, ForeignKey('t2.id')))
+        t2 = Table('t2', metadata, Column('id', Integer, primary_key=True), Column('data', String(50)), Column('t3id', Integer, ForeignKey('t3.id')))
+        t3 = Table('t3', metadata, Column('id', Integer, primary_key=True), Column('data', String(50)))
+
+    def setup_mappers(self):
+        global T1, T2, T3
+        class T1(fixtures.Base):pass
+        class T2(fixtures.Base):pass
+        class T3(fixtures.Base):pass
+
+        mapper(T1, t1, properties={'t2':relation(T2, cascade="all, delete-orphan")})
+        mapper(T2, t2, properties={'t3':relation(T3, cascade="all, delete-orphan")})
+        mapper(T3, t3)
+
+    def test_cascade_delete(self):
+        sess = create_session()
+
+        x = T1(data='t1a', t2=T2(data='t2a', t3=T3(data='t3a')))
+        sess.save(x)
+        sess.flush()
+
+        sess.delete(x)
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [])
+        self.assertEquals(sess.query(T3).all(), [])
+
+    def test_deletes_orphans_onelevel(self):
+        sess = create_session()
+
+        x2 = T1(data='t1b', t2=T2(data='t2b', t3=T3(data='t3b')))
+        sess.save(x2)
+        sess.flush()
+        x2.t2 = None
+
+        sess.delete(x2)
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [])
+        self.assertEquals(sess.query(T3).all(), [])
+
+    def test_deletes_orphans_twolevel(self):
+        sess = create_session()
+
+        x = T1(data='t1a', t2=T2(data='t2a', t3=T3(data='t3a')))
+        sess.save(x)
+        sess.flush()
+
+        x.t2.t3 = None
+        sess.delete(x)
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [])
+        self.assertEquals(sess.query(T2).all(), [])
+        self.assertEquals(sess.query(T3).all(), [])
+
+    def test_finds_orphans_twolevel(self):
+        sess = create_session()
+
+        x = T1(data='t1a', t2=T2(data='t2a', t3=T3(data='t3a')))
+        sess.save(x)
+        sess.flush()
+
+        x.t2.t3 = None
+        sess.flush()
+        self.assertEquals(sess.query(T1).all(), [T1()])
+        self.assertEquals(sess.query(T2).all(), [T2()])
+        self.assertEquals(sess.query(T3).all(), [])
+
 class M2MCascadeTest(ORMTest):
     def define_tables(self, metadata):
-        global a, b, atob
+        global a, b, atob, c
         a = Table('a', metadata,
             Column('id', Integer, primary_key=True),
             Column('data', String(30))
@@ -235,9 +460,13 @@ class M2MCascadeTest(ORMTest):
         atob = Table('atob', metadata,
             Column('aid', Integer, ForeignKey('a.id')),
             Column('bid', Integer, ForeignKey('b.id'))
-
             )
-
+        c = Table('c', metadata, 
+            Column('id', Integer, primary_key=True),
+            Column('data', String(30)),
+            Column('bid', Integer, ForeignKey('b.id'))
+            )
+            
     def test_delete_orphan(self):
         class A(fixtures.Base):
             pass
@@ -262,6 +491,34 @@ class M2MCascadeTest(ORMTest):
         assert b.count().scalar() == 0
         assert a.count().scalar() == 1
 
+    def test_delete_orphan_cascades(self):
+        class A(fixtures.Base):
+            pass
+        class B(fixtures.Base):
+            pass
+        class C(fixtures.Base):
+            pass
+
+        mapper(A, a, properties={
+            # if no backref here, delete-orphan failed until [ticket:427] was fixed
+            'bs':relation(B, secondary=atob, cascade="all, delete-orphan")
+        })
+        mapper(B, b, properties={'cs':relation(C, cascade="all, delete-orphan")})
+        mapper(C, c)
+
+        sess = create_session()
+        b1 = B(data='b1', cs=[C(data='c1')])
+        a1 = A(data='a1', bs=[b1])
+        sess.save(a1)
+        sess.flush()
+
+        a1.bs.remove(b1)
+        sess.flush()
+        assert atob.count().scalar() ==0
+        assert b.count().scalar() == 0
+        assert a.count().scalar() == 1
+        assert c.count().scalar() == 0
+        
     def test_cascade_delete(self):
         class A(fixtures.Base):
             pass

diff --git a/test/orm/dynamic.py b/test/orm/dynamic.py
index 2b5c0eeecb701580e8169a9777caccb22439fd13..2625a967eff136703e6c8936adba560734580a05 100644 (file)
--- a/test/orm/dynamic.py
+++ b/test/orm/dynamic.py
@@ -91,7 +91,17 @@ class DynamicTest(FixtureTest):
 
         assert o1 in i1.orders.all()
         assert i1 in o1.items.all()
-
+    
+    def test_transient_detached(self):
+        mapper(User, users, properties={
+            'addresses':dynamic_loader(mapper(Address, addresses))
+        })
+        sess = create_session()
+        u1 = User()
+        u1.addresses.append(Address())
+        assert u1.addresses.count() == 1
+        assert u1.addresses[0] == Address()
+        
 class FlushTest(FixtureTest):
     def test_basic(self):
         class Fixture(Base):