From: Mike Bayer Date: Tue, 18 Mar 2008 20:59:52 +0000 (+0000) Subject: - fixed/added coverage for various cascade scenarios X-Git-Tag: rel_0_4_5~76 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=13475937e4cdbd1ffaa98d2c426dd118f7edd74f;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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 be2e1eb10d..3fb68b1ef1 100644 --- 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 8b41d93dd9..425b171df9 100644 --- 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 ae55b4c943..1676eeb22c 100644 --- 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 1abf01de77..7a68a4d58a 100644 --- 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 2b5c0eeecb..2625a967ef 100644 --- 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):