- "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 4b5b2e5d23502a161b66ae66a2a6ca7828b759d7..1949a7f3706eb039f976f6c5d7943a71ad3869f5 100644 (file)
--- 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 0a0274c46aacec1bde41a78841e70ec8b2083a64..df084eb2522b872daf7a3bda32ea1850a2a523e4 100644 (file)
--- 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 3327f13c34a9cf360f0cd2202deb7bc319e67fb0..e07f691f41d1e83ebc1cd392055b3985653be629 100644 (file)
--- 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 6e5590bb00ae43ac7522e2735ba0ec79bcb4b7f0..c4b75f64441b4ebeb4d41385fe1460be5058abc4 100644 (file)
--- 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 0fbf5818b370c6bf8a4033ca050c3d49be1bb8e6..fb163ccf05e5a57c4ecc9fd4608dfe223d7e57a0 100644 (file)
--- 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()