- further changes to attributes with regards to "trackparent". the "commit" operation

now sets a "hasparent" flag for all attributes to all objects.  that way lazy loads
via callables get included in trackparent, and eager loads do as well because the mapper
calls commit() on all objects at load time.  this is a less shaky method than the "optimistic"
thing in the previous commit, but uses more memory and involves more overhead.
- some tweaks/cleanup to unit tests

3 files changed, 30 insertions, 23 deletions

diff --git a/lib/sqlalchemy/attributes.py b/lib/sqlalchemy/attributes.py
index 8629d85a5..2a98a5f4d 100644
--- a/lib/sqlalchemy/attributes.py
+++ b/lib/sqlalchemy/attributes.py
@@ -32,19 +32,16 @@ class InstrumentedAttribute(object):
         return self.get(obj)
 
     def hasparent(self, item, optimistic=False):
-        """return True if the given item is attached to a parent object 
-        via the attribute represented by this InstrumentedAttribute.
-        
-        optimistic indicates what we should return if the given item has no "hasparent"
-        record at all for the given attribute."""
-        return item._state.get(('hasparent', id(self)), optimistic)
+        """return the boolean value of a "hasparent" flag attached to the given item.
+        """
+        return item._state.get(('hasparent', id(self)), False)
         
     def sethasparent(self, item, value):
         """sets a boolean flag on the given item corresponding to whether or not it is
         attached to a parent object via the attribute represented by this InstrumentedAttribute."""
         if item is not None:
             item._state[('hasparent', id(self))] = value
-
+            
     def get_history(self, obj, passive=False):
         """return a new AttributeHistory object for the given object/this attribute's key.
         
@@ -140,16 +137,12 @@ class InstrumentedAttribute(object):
                     values = callable_()
                     l = InstrumentedList(self, obj, self._adapt_list(values), init=False)
                     
-                    # mark loaded instances with "hasparent" status.  commented out
-                    # because loaded objects use "optimistic" parent-checking
-                    #if self.trackparent and values is not None:
-                    #    [self.sethasparent(v, True) for v in values if v is not None]
-                    
                     # if a callable was executed, then its part of the "committed state"
                     # if any, so commit the newly loaded data
                     orig = state.get('original', None)
                     if orig is not None:
                         orig.commit_attribute(self, obj, l)
+                    
                 else:
                     # note that we arent raising AttributeErrors, just creating a new
                     # blank list and setting it.
@@ -165,11 +158,6 @@ class InstrumentedAttribute(object):
                     value = callable_()
                     obj.__dict__[self.key] = value
 
-                    # mark loaded instances with "hasparent" status.  commented out
-                    # because loaded objects use "optimistic" parent-checking
-                    #if self.trackparent and value is not None:
-                    #    self.sethasparent(value, True)
-                    
                     # if a callable was executed, then its part of the "committed state"
                     # if any, so commit the newly loaded data
                     orig = state.get('original', None)
@@ -478,14 +466,21 @@ class CommittedState(object):
         if attr.uselist:
             if value is not False:
                 self.data[attr.key] = [x for x in value]
+                if attr.trackparent:
+                    [attr.sethasparent(x, True) for x in self.data[attr.key]]
             elif obj.__dict__.has_key(attr.key):
                 self.data[attr.key] = [x for x in obj.__dict__[attr.key]]
+                if attr.trackparent:
+                    [attr.sethasparent(x, True) for x in self.data[attr.key]]
         else:
             if value is not False:
                 self.data[attr.key] = value
+                if attr.trackparent:
+                    attr.sethasparent(self.data[attr.key], True)
             elif obj.__dict__.has_key(attr.key):
                 self.data[attr.key] = obj.__dict__[attr.key]
-                        
+                if attr.trackparent:
+                    attr.sethasparent(self.data[attr.key], True)
     def rollback(self, manager, obj):
         for attr in manager.managed_attributes(obj.__class__):
             if self.data.has_key(attr.key):
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 21d4c6e36..c7531eb8e 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -141,9 +141,10 @@ class Mapper(object):
         #self.compile()
     
     def _is_orphan(self, obj):
-        optimistic = hasattr(obj, '_instance_key')
         for (key,klass) in self.delete_orphans:
-            if not getattr(klass, key).hasparent(obj, optimistic=optimistic):
+            if not getattr(klass, key).hasparent(obj):
+                if not has_identity(obj):
+                    raise exceptions.FlushError("instance %s is an unsaved, pending instance and is an orphan" % obj)
                 return True
         else:
             return False
@@ -710,7 +711,7 @@ class Mapper(object):
 
         if not postupdate:
             for obj in objects:
-                if not hasattr(obj, "_instance_key"):
+                if not has_identity(obj):
                     self.extension.before_insert(self, connection, obj)
                 else:
                     self.extension.before_update(self, connection, obj)
@@ -747,7 +748,7 @@ class Mapper(object):
                 # 'postupdate' means a PropertyLoader is telling us, "yes I know you 
                 # already inserted/updated this row but I need you to UPDATE one more 
                 # time"
-                isinsert = not postupdate and not hasattr(obj, "_instance_key")
+                isinsert = not postupdate and not has_identity(obj)
                 hasdata = False
                 for col in table.columns:
                     if col is self.version_id_col:
@@ -1392,6 +1393,9 @@ def hash_key(obj):
     else:
         return repr(obj)
 
+def has_identity(object):
+    return hasattr(object, '_instance_key')
+    
 def has_mapper(object):
     """returns True if the given object has a mapper association"""
     return hasattr(object, '_entity_name')
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 16760dc06..e8f3ad7dd 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -218,7 +218,7 @@ class PropertyLoader(mapper.MapperProperty):
 
         if self.cascade.delete_orphan:
             if self.parent.class_ is self.mapper.class_:
-                raise exceptions.ArgumentError("Cant establish 'delete-orphan' cascade rule on a self-referential relationship.  You probably want cascade='all', which includes delete cascading but not orphan detection.")
+                raise exceptions.ArgumentError("Cant establish 'delete-orphan' cascade rule on a self-referential relationship (attribute '%s' on class '%s').  You probably want cascade='all', which includes delete cascading but not orphan detection." %(self.key, self.parent.class_.__name__))
             self.mapper.primary_mapper().delete_orphans.append((self.key, self.parent.class_))
             
         if self.secondaryjoin is not None and self.secondary is None:
@@ -379,9 +379,17 @@ class LazyLoader(PropertyLoader):
                 return None
             else:
                 return mapper.object_mapper(instance).props[self.key].setup_loader(instance)
+        
         def lazyload():
             params = {}
             allparams = True
+            # if the instance wasnt loaded from the database, then it cannot lazy load
+            # child items.  one reason for this is that a bi-directional relationship
+            # will not update properly, since bi-directional uses lazy loading functions
+            # in both directions, and this instance will not be present in the lazily-loaded
+            # results of the other objects since its not in the database
+            if not mapper.has_identity(instance):
+                return None
             #print "setting up loader, lazywhere", str(self.lazywhere), "binds", self.lazybinds
             for col, bind in self.lazybinds.iteritems():
                 params[bind.key] = self.parent._getattrbycolumn(instance, col)