From: Mike Bayer Date: Fri, 10 Nov 2006 00:46:57 +0000 (+0000) Subject: - "delete-orphan" for a certain type can be set on more than one parent class; X-Git-Tag: rel_0_3_1~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4cbbf7725a565267b96bf003163713ef7f98b210;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - "delete-orphan" for a certain type can be set on more than one parent class; the instance is an "orphan" only if its not attached to *any* of those parents - better check for endless recursion in eagerloader.process_row --- diff --git a/CHANGES b/CHANGES index 4b5b2e5d23..1949a7f370 100644 --- a/CHANGES +++ b/CHANGES @@ -28,6 +28,8 @@ mappings. new example examples/association/proxied_association.py illustrates. the target class - fix to subtle condition in topological sort where a node could appear twice, for [ticket:362] +- "delete-orphan" for a certain type can be set on more than one parent class; +the instance is an "orphan" only if its not attached to *any* of those parents 0.3.0 - General: diff --git a/doc/build/content/adv_datamapping.txt b/doc/build/content/adv_datamapping.txt index 0a0274c46a..df084eb252 100644 --- a/doc/build/content/adv_datamapping.txt +++ b/doc/build/content/adv_datamapping.txt @@ -179,7 +179,7 @@ Many to many relationships can be customized by one or both of `primaryjoin` and pass mapper(Keyword, keywords_table) mapper(User, users_table, properties={ - 'keywords':relation(Keyword, secondary=userkeywords_table + 'keywords':relation(Keyword, secondary=userkeywords_table, primaryjoin=users_table.c.user_id==userkeywords_table.c.user_id, secondaryjoin=userkeywords_table.c.keyword_id==keywords_table.c.keyword_id ) diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 3327f13c34..e07f691f41 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -220,12 +220,20 @@ class Mapper(object): def _is_orphan(self, obj): optimistic = has_identity(obj) for (key,klass) in self.delete_orphans: - if not getattr(klass, key).hasparent(obj, optimistic=optimistic): - if not has_identity(obj): - raise exceptions.FlushError("instance %s is an unsaved, pending instance and is an orphan (is not attached to any parent '%s' instance via that classes' '%s' attribute)" % (obj, klass.__name__, key)) - return True + if getattr(klass, key).hasparent(obj, optimistic=optimistic): + return False else: - return False + if len(self.delete_orphans): + if not has_identity(obj): + raise exceptions.FlushError("instance %s is an unsaved, pending instance and is an orphan (is not attached to %s)" % + ( + obj, + ", nor ".join(["any parent '%s' instance via that classes' '%s' attribute" % (klass.__name__, key) for (key,klass) in self.delete_orphans]) + )) + else: + return True + else: + return False def _get_props(self): self.compile() diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 6e5590bb00..c4b75f6444 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -470,7 +470,6 @@ class EagerLoader(AbstractRelationLoader): else: decorated_row = decorator(row) else: - # AliasedClauses, keyed to the lead mapper used in the query clauses = self.clauses_by_lead_mapper[selectcontext.mapper] decorated_row = clauses._decorate_row(row) # check for identity key @@ -481,36 +480,36 @@ class EagerLoader(AbstractRelationLoader): self.parent_property._get_strategy(LazyLoader).process_row(selectcontext, instance, row, identitykey, isnew) return - if not self.uselist: - self.logger.debug("eagerload scalar instance on %s" % mapperutil.attribute_str(instance, self.key)) - if isnew: - # set a scalar object instance directly on the parent object, - # bypassing SmartProperty event handlers. - instance.__dict__[self.key] = self.mapper._instance(selectcontext, decorated_row, None) + # TODO: recursion check a speed hit...? try to get a "termination point" into the AliasedClauses + # or EagerRowAdapter ? + selectcontext.recursion_stack.add(self) + try: + if not self.uselist: + self.logger.debug("eagerload scalar instance on %s" % mapperutil.attribute_str(instance, self.key)) + if isnew: + # set a scalar object instance directly on the parent object, + # bypassing SmartProperty event handlers. + instance.__dict__[self.key] = self.mapper._instance(selectcontext, decorated_row, None) + else: + # call _instance on the row, even though the object has been created, + # so that we further descend into properties + self.mapper._instance(selectcontext, decorated_row, None) else: - # call _instance on the row, even though the object has been created, - # so that we further descend into properties - self.mapper._instance(selectcontext, decorated_row, None) - else: - if isnew: - self.logger.debug("initialize UniqueAppender on %s" % mapperutil.attribute_str(instance, self.key)) - # call the SmartProperty's initialize() method to create a new, blank list - l = getattr(instance.__class__, self.key).initialize(instance) + if isnew: + self.logger.debug("initialize UniqueAppender on %s" % mapperutil.attribute_str(instance, self.key)) + # call the SmartProperty's initialize() method to create a new, blank list + l = getattr(instance.__class__, self.key).initialize(instance) - # create an appender object which will add set-like semantics to the list - appender = util.UniqueAppender(l.data) + # create an appender object which will add set-like semantics to the list + appender = util.UniqueAppender(l.data) - # store it in the "scratch" area, which is local to this load operation. - selectcontext.attributes[(instance, self.key)] = appender - result_list = selectcontext.attributes[(instance, self.key)] - self.logger.debug("eagerload list instance on %s" % mapperutil.attribute_str(instance, self.key)) - # TODO: recursion check a speed hit...? try to get a "termination point" into the AliasedClauses - # or EagerRowAdapter ? - selectcontext.recursion_stack.add(self) - try: + # store it in the "scratch" area, which is local to this load operation. + selectcontext.attributes[(instance, self.key)] = appender + result_list = selectcontext.attributes[(instance, self.key)] + self.logger.debug("eagerload list instance on %s" % mapperutil.attribute_str(instance, self.key)) self.mapper._instance(selectcontext, decorated_row, result_list) - finally: - selectcontext.recursion_stack.remove(self) + finally: + selectcontext.recursion_stack.remove(self) EagerLoader.logger = logging.class_logger(EagerLoader) diff --git a/test/orm/session.py b/test/orm/session.py index 0fbf5818b3..fb163ccf05 100644 --- a/test/orm/session.py +++ b/test/orm/session.py @@ -165,11 +165,71 @@ class CascadingOrphanDeletionTest(AssertMixin): try: s.flush() assert False - except exceptions.FlushError: + except exceptions.FlushError, e: + print e assert True assert item.id is None assert attr.id is None +class DoubleOrphanTest(testbase.AssertMixin): + def setUpAll(self): + global metadata, address_table, businesses, homes + metadata = BoundMetaData(testbase.db) + address_table = Table('addresses', metadata, + Column('address_id', Integer, primary_key=True), + Column('street', String(30)), + ) + + homes = Table('homes', metadata, + Column('home_id', Integer, primary_key=True), + Column('description', String(30)), + Column('address_id', Integer, ForeignKey('addresses.address_id'), nullable=False), + ) + + businesses = Table('businesses', metadata, + Column('business_id', Integer, primary_key=True, key="id"), + Column('description', String(30), key="description"), + Column('address_id', Integer, ForeignKey('addresses.address_id'), nullable=False), + ) + metadata.create_all() + def tearDown(self): + clear_mappers() + def tearDownAll(self): + metadata.drop_all() + def test_non_orphan(self): + class Address(object):pass + class Home(object):pass + class Business(object):pass + mapper(Address, address_table) + mapper(Home, homes, properties={'address':relation(Address, cascade="all,delete-orphan")}) + mapper(Business, businesses, properties={'address':relation(Address, cascade="all,delete-orphan")}) + + session = create_session() + a1 = Address() + a2 = Address() + h1 = Home() + b1 = Business() + h1.address = a1 + b1.address = a2 + [session.save(x) for x in [h1,b1]] + session.flush() + def test_orphan(self): + class Address(object):pass + class Home(object):pass + class Business(object):pass + mapper(Address, address_table) + mapper(Home, homes, properties={'address':relation(Address, cascade="all,delete-orphan")}) + mapper(Business, businesses, properties={'address':relation(Address, cascade="all,delete-orphan")}) + + session = create_session() + a1 = Address() + session.save(a1) + try: + session.flush() + assert False + except exceptions.FlushError, e: + assert True + if __name__ == "__main__": testbase.main()