futher fix to the "orphan state" idea. to avoid setting tons of

"hasparent" flags on objects as they are loaded, both from lazy and eager loads,
the "orphan" check now uses an "optimistic" flag to determine the result if no
"hasparent" flag is found for a particular relationship on an instance. if the
instance has an _instance_key and therefore was loaded from the database, it is
assumed to not be an orphan unless a "False" hasparent flag has been set.  if the
instance does not have an _instance_key and is therefore transient/pending, it is
assumed to be an orphan unless a "True" hasparent flag has been set.

diff --git a/CHANGES b/CHANGES
index c509caeb6dcecf5b458fac6f732d957f36b6517f..79d8e3d6d5f29cea44a0db2d87a85a7c3fcd9fa6 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -26,6 +26,13 @@ persistent with a session already.
 - unit-of-work does a better check for "orphaned" objects that are
 part of a "delete-orphan" cascade, for certain conditions where the 
 parent isnt available to cascade from.
+- mappers can tell if one of their objects is an "orphan" based
+on interactions with the attribute package. this check is based
+on a status flag maintained for each relationship 
+when objects are attached and detached from each other.  if the
+status flag is not present, its assumed to be "False" for a 
+transient instance and assumed to be "True" for a persisted/detached
+ instance.
 - it is now invalid to declare a self-referential relationship with
 "delete-orphan" (as the abovementioned check would make them impossible
 to save)

diff --git a/lib/sqlalchemy/attributes.py b/lib/sqlalchemy/attributes.py
index 3aada1951a3ab2fb730cda12950bf1e81ff927ab..8629d85a50c253908c5678507b96a4e8a9d38490 100644 (file)
--- a/lib/sqlalchemy/attributes.py
+++ b/lib/sqlalchemy/attributes.py
@@ -31,10 +31,13 @@ class InstrumentedAttribute(object):
             return self
         return self.get(obj)
 
-    def hasparent(self, item):
-        """returns True if the given item is attached to a parent object 
-        via the attribute represented by this InstrumentedAttribute."""
-        return item._state.get(('hasparent', id(self)))
+    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)
         
     def sethasparent(self, item, value):
         """sets a boolean flag on the given item corresponding to whether or not it is
@@ -136,8 +139,12 @@ class InstrumentedAttribute(object):
                         return InstrumentedAttribute.PASSIVE_NORESULT
                     values = callable_()
                     l = InstrumentedList(self, obj, self._adapt_list(values), init=False)
-                    if self.trackparent and values is not None:
-                        [self.sethasparent(v, True) for v in values if v is not None]
+                    
+                    # 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)
@@ -157,8 +164,12 @@ class InstrumentedAttribute(object):
                         return InstrumentedAttribute.PASSIVE_NORESULT
                     value = callable_()
                     obj.__dict__[self.key] = value
-                    if self.trackparent and value is not None:
-                        self.sethasparent(value, True)
+
+                    # 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)

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 5fed12188835999f51a7af53cae87e109e76aaa8..21d4c6e36713997b8a7af721b1d165870dfc0e20 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -141,8 +141,9 @@ 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):
+            if not getattr(klass, key).hasparent(obj, optimistic=optimistic):
                 return True
         else:
             return False

diff --git a/test/base/attributes.py b/test/base/attributes.py
index f281e487b5e676134a7c555dc30f465382c710b0..831848cd994596fc3c3ae5fba3d8778070f8cd37 100644 (file)
--- a/test/base/attributes.py
+++ b/test/base/attributes.py
@@ -186,10 +186,22 @@ class AttributesTest(PersistTest):
         assert p1.blog is b
         assert p1 in b.posts
 
-        # no orphans
-        assert getattr(Blog, 'posts').hasparent(p1)
-        assert getattr(Post, 'blog').hasparent(b)
+        # no orphans (but we are using optimistic checks)
+        assert getattr(Blog, 'posts').hasparent(p1, optimistic=True)
+        assert getattr(Post, 'blog').hasparent(b, optimistic=True)
         
+        # lazy loads currently not processed for "hasparent" status, so without
+        # optimistic, it returns false
+        assert not getattr(Blog, 'posts').hasparent(p1, optimistic=False)
+        assert not getattr(Post, 'blog').hasparent(b, optimistic=False)
+        
+        # ok what about non-optimistic.  well, dont use lazy loaders,
+        # assign things manually, so the "hasparent" flags get set
+        b2 = Blog()
+        p2 = Post()
+        b2.posts.append(p2)
+        assert getattr(Blog, 'posts').hasparent(p2, optimistic=False)
+        assert getattr(Post, 'blog').hasparent(b2, optimistic=False)
         
     def testinheritance(self):
         """tests that attributes are polymorphic"""

diff --git a/test/orm/mapper.py b/test/orm/mapper.py
index 2704e8e7ccd6c4d3b01f73dcfc30372b621d3d45..e12e15139ec44c35468c05e90a41c03472b0ac59 100644 (file)
--- a/test/orm/mapper.py
+++ b/test/orm/mapper.py
@@ -670,6 +670,18 @@ class LazyTest(MapperSuperTest):
             {'user_id' : 9, 'addresses' : (Address, [])},
             )
 
+    def testorphanstate(self):
+        """test that a lazily loaded child object is not marked as an orphan"""
+        m = mapper(User, users, properties={
+            'addresses':relation(Address, cascade="all,delete-orphan", lazy=True)
+        })
+        mapper(Address, addresses)
+
+        q = create_session().query(m)
+        user = q.get(7)
+        assert getattr(User, 'addresses').hasparent(user.addresses[0], optimistic=True)
+        assert not class_mapper(Address)._is_orphan(user.addresses[0])
+        
     def testlimit(self):
         ordermapper = mapper(Order, orders, properties = dict(
                 items = relation(mapper(Item, orderitems), lazy = True)
@@ -881,6 +893,19 @@ class EagerTest(MapperSuperTest):
             },
         )
 
+    def testorphanstate(self):
+        """test that an eagerly loaded child object is not marked as an orphan"""
+        m = mapper(User, users, properties={
+            'addresses':relation(Address, cascade="all,delete-orphan", lazy=False)
+        })
+        mapper(Address, addresses)
+        
+        s = create_session()
+        q = s.query(m)
+        user = q.get(7)
+        assert getattr(User, 'addresses').hasparent(user.addresses[0], optimistic=True)
+        assert not class_mapper(Address)._is_orphan(user.addresses[0])
+        
     def testwithrepeat(self):
         """tests a one-to-many eager load where we also query on joined criterion, where the joined
         criterion is using the same tables that are used within the eager load.  the mapper must insure that the