From 091d40344ebeedd4633841146449ac772e33b631 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 16 Jun 2006 03:19:53 +0000 Subject: [PATCH] fixed bug where if a many-to-many table mapped as "secondary" had other cols in it, delete operations would try to match up on those columns. also fixed bug in new attributes if you set a list based attribute to a blank list, properly fires the 'delete' event for the elements of the previous list --- CHANGES | 2 + lib/sqlalchemy/attributes.py | 10 +-- lib/sqlalchemy/orm/dependency.py | 2 +- test/orm/objectstore.py | 118 ++++++++++++++++++++----------- test/tables.py | 2 +- 5 files changed, 86 insertions(+), 48 deletions(-) diff --git a/CHANGES b/CHANGES index 020c5bf507..f23b43ab35 100644 --- a/CHANGES +++ b/CHANGES @@ -19,6 +19,8 @@ the MetaData object properly - fixed bug where Column with redefined "key" property wasnt getting type conversion happening in the ResultProxy [ticket:207] - fixed 'port' attribute of URL to be an integer if present +- fixed old bug where if a many-to-many table mapped as "secondary" +had extra columns, delete operations didnt work 0.2.2 - big improvements to polymorphic inheritance behavior, enabling it diff --git a/lib/sqlalchemy/attributes.py b/lib/sqlalchemy/attributes.py index a9b3134010..60cbecec14 100644 --- a/lib/sqlalchemy/attributes.py +++ b/lib/sqlalchemy/attributes.py @@ -167,8 +167,7 @@ class InstrumentedAttribute(object): trig() if self.uselist: value = InstrumentedList(self, obj, value) - elif self.trackparent or len(self.extensions): - old = self.get(obj) + old = self.get(obj) obj.__dict__[self.key] = value state['modified'] = True if not self.uselist: @@ -179,7 +178,10 @@ class InstrumentedAttribute(object): self.sethasparent(old, False) for ext in self.extensions: ext.set(event or self, obj, value, old) - + else: + # set the deleted event for the old item + old[:] = [] + def delete(self, event, obj): """deletes a value from the given object. 'event' is the InstrumentedAttribute that initiated the delete() operation and is used to control the depth of a circular delete @@ -499,7 +501,7 @@ class AttributeHistory(object): else: self._deleted_items = [] self._unchanged_items = [] - + #print "orig", original, "current", current, "added", self._added_items, "unchanged", self._unchanged_items, "deleted", self._deleted_items def __iter__(self): return iter(self._current) def added_items(self): diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 145ec5e9b6..067e842c35 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -281,7 +281,7 @@ class ManyToManyDP(DependencyProcessor): if len(secondary_delete): # TODO: precompile the delete/insert queries and store them as instance variables # on the PropertyLoader - statement = self.secondary.delete(sql.and_(*[c == sql.bindparam(c.key) for c in self.secondary.c])) + statement = self.secondary.delete(sql.and_(*[c == sql.bindparam(c.key) for c in self.secondary.c if c.key in associationrow])) connection.execute(statement, secondary_delete) if len(secondary_insert): statement = self.secondary.insert() diff --git a/test/orm/objectstore.py b/test/orm/objectstore.py index d267520f3d..e897d84a8a 100644 --- a/test/orm/objectstore.py +++ b/test/orm/objectstore.py @@ -920,54 +920,15 @@ class SaveTest(SessionTest): a1.user = None ctx.current.flush() - def _testalias(self): - """tests that an alias of a table can be used in a mapper. - the mapper has to locate the original table and columns to keep it all straight.""" - ualias = Alias(users, 'ualias') - m = mapper(User, ualias) - u = User() - u.user_name = 'testalias' - m.save(u) - - u2 = m.select(ualias.c.user_id == u.user_id)[0] - self.assert_(u2 is u) - - def _testremove(self): - m = mapper(User, users, properties = dict( - addresses = relation(mapper(Address, addresses), lazy = True) - )) - u = User() - u.user_name = 'one2manytester' - u.addresses = [] - a = Address() - a.email_address = 'one2many@test.org' - u.addresses.append(a) - a2 = Address() - a2.email_address = 'lala@test.org' - u.addresses.append(a2) - m.save(u) - addresstable = addresses.select(addresses.c.address_id.in_(a.address_id, a2.address_id)).execute().fetchall() - self.echo( repr(addresstable[0].values())) - self.assertEqual(addresstable[0].values(), [a.address_id, u.user_id, 'one2many@test.org']) - self.assertEqual(addresstable[1].values(), [a2.address_id, u.user_id, 'lala@test.org']) - del u.addresses[1] - m.save(u) - addresstable = addresses.select(addresses.c.address_id.in_(a.address_id, a2.address_id)).execute().fetchall() - self.echo( repr(addresstable)) - self.assertEqual(addresstable[0].values(), [a.address_id, u.user_id, 'one2many@test.org']) - self.assertEqual(addresstable[1].values(), [a2.address_id, None, 'lala@test.org']) - def testmanytomany(self): items = orderitems keywordmapper = mapper(Keyword, keywords) - items.select().execute() m = mapper(Item, items, properties = dict( keywords = relation(keywordmapper, itemkeywords, lazy = False), )) - data = [Item, {'item_name': 'mm_item1', 'keywords' : (Keyword,[{'name': 'big'},{'name': 'green'}, {'name': 'purple'},{'name': 'round'}])}, {'item_name': 'mm_item2', 'keywords' : (Keyword,[{'name':'blue'}, {'name':'imnew'},{'name':'round'}, {'name':'small'}])}, @@ -1049,8 +1010,30 @@ class SaveTest(SessionTest): ctx.current.delete(objects[3]) ctx.current.flush() + def testmanytomanyremove(self): + """tests that setting a list-based attribute to '[]' properly affects the history and allows + the many-to-many rows to be deleted""" + keywordmapper = mapper(Keyword, keywords) + + m = mapper(Item, orderitems, properties = dict( + keywords = relation(keywordmapper, itemkeywords, lazy = False), + )) + + i = Item() + k1 = Keyword() + k2 = Keyword() + i.keywords.append(k1) + i.keywords.append(k2) + ctx.current.flush() + + assert itemkeywords.count().scalar() == 2 + i.keywords = [] + ctx.current.flush() + assert itemkeywords.count().scalar() == 0 + def testmanytomanyupdate(self): + """tests some history operations on a many to many""" class Keyword(object): def __init__(self, name): self.name = name @@ -1075,9 +1058,6 @@ class SaveTest(SessionTest): item.keywords = [] item.keywords.append(k1) item.keywords.append(k2) - print item.keywords.unchanged_items() - print item.keywords.added_items() - print item.keywords.deleted_items() ctx.current.flush() ctx.current.clear() @@ -1086,6 +1066,7 @@ class SaveTest(SessionTest): assert item.keywords == [k1, k2] def testassociation(self): + """basic test of an association object""" class IKAssociation(object): def __repr__(self): return "\nIKAssociation " + repr(self.item_id) + " " + repr(self.keyword) @@ -1283,6 +1264,59 @@ class SaveTest2(SessionTest): ] ) +class SaveTest3(SessionTest): + + def setUpAll(self): + SessionTest.setUpAll(self) + global metadata, t1, t2, t3 + metadata = testbase.metadata + t1 = Table('items', metadata, + Column('item_id', INT, Sequence('items_id_seq', optional=True), primary_key = True), + Column('item_name', VARCHAR(50)), + ) + + t3 = Table('keywords', metadata, + Column('keyword_id', Integer, Sequence('keyword_id_seq', optional=True), primary_key = True), + Column('name', VARCHAR(50)), + + ) + t2 = Table('assoc', metadata, + Column('item_id', INT, ForeignKey("items")), + Column('keyword_id', INT, ForeignKey("keywords")), + Column('foo', Boolean, default=True) + ) + metadata.create_all() + def tearDownAll(self): + metadata.drop_all() + SessionTest.tearDownAll(self) + + def setUp(self): + pass + def tearDown(self): + pass + + def testmanytomanyxtracolremove(self): + """tests that a many-to-many on a table that has an extra column can properly delete rows from the table + without referencing the extra column""" + mapper(Keyword, t3) + + mapper(Item, t1, properties = dict( + keywords = relation(Keyword, secondary=t2, lazy = False), + )) + + i = Item() + k1 = Keyword() + k2 = Keyword() + i.keywords.append(k1) + i.keywords.append(k2) + ctx.current.flush() + + assert t2.count().scalar() == 2 + i.keywords = [] + print i.keywords + ctx.current.flush() + assert t2.count().scalar() == 0 + if __name__ == "__main__": testbase.main() diff --git a/test/tables.py b/test/tables.py index 7085e5a775..72870ec1eb 100644 --- a/test/tables.py +++ b/test/tables.py @@ -53,7 +53,7 @@ userkeywords = Table('userkeywords', metadata, itemkeywords = Table('itemkeywords', metadata, Column('item_id', INT, ForeignKey("items")), Column('keyword_id', INT, ForeignKey("keywords")), - +# Column('foo', Boolean, default=True) ) def create(): -- 2.47.2