- added some abstraction to the attributes.History object

- Repaired support for "passive-deletes" on a many-to-one
relation() with "delete" cascade. [ticket:1183]

8 files changed, 140 insertions, 95 deletions

diff --git a/CHANGES b/CHANGES
index fbc6963d4..b022671dc 100644
--- a/CHANGES
+++ b/CHANGES
@@ -32,6 +32,9 @@ CHANGES
     - polymorphic_union() function respects the "key" of each Column
       if they differ from the column's name.
 
+    - Repaired support for "passive-deletes" on a many-to-one
+      relation() with "delete" cascade. [ticket:1183]
+
     - Added more granularity to internal attribute access, such that
       cascade and flush operations will not initialize unloaded
       attributes and collections, leaving them intact for a
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index bc8f701e7..36a532faa 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -511,7 +511,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
         else:
             current = self.get(state, passive=passive)
             if current is PASSIVE_NORESULT:
-                return (None, None, None)
+                return HISTORY_BLANK
             else:
                 return History.from_attribute(self, state, current)
 
@@ -590,7 +590,7 @@ class CollectionAttributeImpl(AttributeImpl):
     def get_history(self, state, passive=PASSIVE_OFF):
         current = self.get(state, passive=passive)
         if current is PASSIVE_NORESULT:
-            return (None, None, None)
+            return HISTORY_BLANK
         else:
             return History.from_attribute(self, state, current)
 
@@ -1387,7 +1387,29 @@ class History(tuple):
 
     def __new__(cls, added, unchanged, deleted):
         return tuple.__new__(cls, (added, unchanged, deleted))
-
+    
+    def __nonzero__(self):
+        return self != HISTORY_BLANK
+    
+    def sum(self):
+        return self.added + self.unchanged + self.deleted
+    
+    def non_deleted(self):
+        return self.added + self.unchanged
+    
+    def non_added(self):
+        return self.unchanged + self.deleted
+    
+    def has_changes(self):
+        return bool(self.added or self.deleted)
+        
+    def as_state(self):
+        return History(
+            [c is not None and instance_state(c) or None for c in self.added],
+            [c is not None and instance_state(c) or None for c in self.unchanged],
+            [c is not None and instance_state(c) or None for c in self.deleted],
+        )
+    
     @classmethod
     def from_attribute(cls, attribute, state, current):
         original = state.committed_state.get(attribute.key, NEVER_SET)
@@ -1424,6 +1446,7 @@ class History(tuple):
                     deleted = ()
                 return cls([current], (), deleted)
 
+HISTORY_BLANK = History(None, None, None)
 
 class PendingCollection(object):
     """A writable placeholder for an unloaded collection.
diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index b33213a80..2d0b7f1c6 100644
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -169,32 +169,32 @@ class OneToManyDP(DependencyProcessor):
             # is on.
             if self.post_update or not self.passive_deletes == 'all':
                 for state in deplist:
-                    (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
-                    if unchanged or deleted:
-                        for child in deleted:
+                    history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
+                    if history:
+                        for child in history.deleted:
                             if child is not None and self.hasparent(child) is False:
                                 self._synchronize(state, child, None, True, uowcommit)
                                 self._conditional_post_update(child, uowcommit, [state])
                         if self.post_update or not self.cascade.delete:
-                            for child in unchanged:
+                            for child in history.unchanged:
                                 if child is not None:
                                     self._synchronize(state, child, None, True, uowcommit)
                                     self._conditional_post_update(child, uowcommit, [state])
         else:
             for state in deplist:
-                (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True)
-                if added or deleted:
-                    for child in added:
+                history = uowcommit.get_attribute_history(state, self.key, passive=True)
+                if history:
+                    for child in history.added:
                         self._synchronize(state, child, None, False, uowcommit)
                         if child is not None:
                             self._conditional_post_update(child, uowcommit, [state])
-                    for child in deleted:
+
+                    for child in history.deleted:
                         if not self.cascade.delete_orphan and not self.hasparent(child):
                             self._synchronize(state, child, None, True, uowcommit)
 
-                if self._pks_changed(uowcommit, state):
-                    if unchanged:
-                        for child in unchanged:
+                    if self._pks_changed(uowcommit, state):
+                        for child in history.unchanged:
                             self._synchronize(state, child, None, False, uowcommit)
 
     def preprocess_dependencies(self, task, deplist, uowcommit, delete = False):
@@ -206,26 +206,26 @@ class OneToManyDP(DependencyProcessor):
             if not self.post_update:
                 should_null_fks = not self.cascade.delete and not self.passive_deletes == 'all'
                 for state in deplist:
-                    (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
-                    if unchanged or deleted:
-                        for child in deleted:
+                    history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
+                    if history:
+                        for child in history.deleted:
                             if child is not None and self.hasparent(child) is False:
                                 if self.cascade.delete_orphan:
                                     uowcommit.register_object(child, isdelete=True)
                                 else:
                                     uowcommit.register_object(child)
                         if should_null_fks:
-                            for child in unchanged:
+                            for child in history.unchanged:
                                 if child is not None:
                                     uowcommit.register_object(child)
         else:
             for state in deplist:
-                (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True)
-                if added or deleted:
-                    for child in added:
+                history = uowcommit.get_attribute_history(state, self.key, passive=True)
+                if history:
+                    for child in history.added:
                         if child is not None:
                             uowcommit.register_object(child)
-                    for child in deleted:
+                    for child in history.deleted:
                         if not self.cascade.delete_orphan:
                             uowcommit.register_object(child, isdelete=False)
                         elif self.hasparent(child) is False:
@@ -235,11 +235,10 @@ class OneToManyDP(DependencyProcessor):
                                     attributes.instance_state(c),
                                     isdelete=True)
                 if not self.passive_updates and self._pks_changed(uowcommit, state):
-                    if not unchanged:
-                        (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=False)
-                    if unchanged:
-                        for child in unchanged:
-                            uowcommit.register_object(child)
+                    if not history:
+                        history = uowcommit.get_attribute_history(state, self.key, passive=False)
+                    for child in history.unchanged:
+                        uowcommit.register_object(child)
 
     def _synchronize(self, state, child, associationrow, clearkeys, uowcommit):
         source = state
@@ -314,7 +313,7 @@ class ManyToOneDP(DependencyProcessor):
             uowcommit.register_processor(self.mapper, self, self.parent)
 
 
-    def process_dependencies(self, task, deplist, uowcommit, delete = False):
+    def process_dependencies(self, task, deplist, uowcommit, delete=False):
         #print self.mapper.mapped_table.name + " " + self.key + " " + repr(len(deplist)) + " process_dep isdelete " + repr(delete) + " direction " + repr(self.direction)
         if delete:
             if self.post_update and not self.cascade.delete_orphan and not self.passive_deletes == 'all':
@@ -322,43 +321,44 @@ class ManyToOneDP(DependencyProcessor):
                 # before we can DELETE the row
                 for state in deplist:
                     self._synchronize(state, None, None, True, uowcommit)
-                    (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
-                    if added or unchanged or deleted:
-                        self._conditional_post_update(state, uowcommit, deleted + unchanged + added)
+                    history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
+                    if history:
+                        self._conditional_post_update(state, uowcommit, history.sum())
         else:
             for state in deplist:
-                (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True)
-                if added or deleted or unchanged:
-                    for child in added:
+                history = uowcommit.get_attribute_history(state, self.key, passive=True)
+                if history:
+                    for child in history.added:
                         self._synchronize(state, child, None, False, uowcommit)
-                    self._conditional_post_update(state, uowcommit, deleted + unchanged + added)
+                    self._conditional_post_update(state, uowcommit, history.sum())
 
-    def preprocess_dependencies(self, task, deplist, uowcommit, delete = False):
+    def preprocess_dependencies(self, task, deplist, uowcommit, delete=False):
         #print self.mapper.mapped_table.name + " " + self.key + " " + repr(len(deplist)) + " PRE process_dep isdelete " + repr(delete) + " direction " + repr(self.direction)
         if self.post_update:
             return
         if delete:
             if self.cascade.delete or self.cascade.delete_orphan:
                 for state in deplist:
-                    (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
-                    if self.cascade.delete_orphan:
-                        todelete = added + unchanged + deleted
-                    else:
-                        todelete = added + unchanged
-                    for child in todelete:
-                        if child is None:
-                            continue
-                        uowcommit.register_object(child, isdelete=True)
-                        for c, m in self.mapper.cascade_iterator('delete', child):
-                            uowcommit.register_object(
-                                attributes.instance_state(c), isdelete=True)
+                    history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
+                    if history:
+                        if self.cascade.delete_orphan:
+                            todelete = history.sum()
+                        else:
+                            todelete = history.non_deleted()
+                        for child in todelete:
+                            if child is None:
+                                continue
+                            uowcommit.register_object(child, isdelete=True)
+                            for c, m in self.mapper.cascade_iterator('delete', child):
+                                uowcommit.register_object(
+                                    attributes.instance_state(c), isdelete=True)
         else:
             for state in deplist:
                 uowcommit.register_object(state)
                 if self.cascade.delete_orphan:
-                    (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
-                    if deleted:
-                        for child in deleted:
+                    history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
+                    if history:
+                        for child in history.deleted:
                             if self.hasparent(child) is False:
                                 uowcommit.register_object(child, isdelete=True)
                                 for c, m in self.mapper.cascade_iterator('delete', child):
@@ -403,9 +403,9 @@ class ManyToManyDP(DependencyProcessor):
 
         if delete:
             for state in deplist:
-                (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
-                if deleted or unchanged:
-                    for child in deleted + unchanged:
+                history = uowcommit.get_attribute_history(state, self.key, passive=self.passive_deletes)
+                if history:
+                    for child in history.non_added():
                         if child is None or (reverse_dep and (reverse_dep, "manytomany", child, state) in uowcommit.attributes):
                             continue
                         associationrow = {}
@@ -414,16 +414,16 @@ class ManyToManyDP(DependencyProcessor):
                         uowcommit.attributes[(self, "manytomany", state, child)] = True
         else:
             for state in deplist:
-                (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key)
-                if added or deleted:
-                    for child in added:
+                history = uowcommit.get_attribute_history(state, self.key)
+                if history:
+                    for child in history.added:
                         if child is None or (reverse_dep and (reverse_dep, "manytomany", child, state) in uowcommit.attributes):
                             continue
                         associationrow = {}
                         self._synchronize(state, child, associationrow, False, uowcommit)
                         uowcommit.attributes[(self, "manytomany", state, child)] = True
                         secondary_insert.append(associationrow)
-                    for child in deleted:
+                    for child in history.deleted:
                         if child is None or (reverse_dep and (reverse_dep, "manytomany", child, state) in uowcommit.attributes):
                             continue
                         associationrow = {}
@@ -432,10 +432,10 @@ class ManyToManyDP(DependencyProcessor):
                         secondary_delete.append(associationrow)
 
                 if not self.passive_updates and self._pks_changed(uowcommit, state):
-                    if not unchanged:
-                        (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=False)
+                    if not history:
+                        history = uowcommit.get_attribute_history(state, self.key, passive=False)
                     
-                    for child in unchanged:
+                    for child in history.unchanged:
                         associationrow = {}
                         sync.update(state, self.parent, associationrow, "old_", self.prop.synchronize_pairs)
                         sync.update(child, self.mapper, associationrow, "old_", self.prop.secondary_synchronize_pairs)
@@ -465,9 +465,9 @@ class ManyToManyDP(DependencyProcessor):
         #print self.mapper.mapped_table.name + " " + self.key + " " + repr(len(deplist)) + " preprocess_dep isdelete " + repr(delete) + " direction " + repr(self.direction)
         if not delete:
             for state in deplist:
-                (added, unchanged, deleted) = uowcommit.get_attribute_history(state, self.key, passive=True)
-                if deleted:
-                    for child in deleted:
+                history = uowcommit.get_attribute_history(state, self.key, passive=True)
+                if history:
+                    for child in history.deleted:
                         if self.cascade.delete_orphan and self.hasparent(child) is False:
                             uowcommit.register_object(child, isdelete=True)
                             for c, m in self.mapper.cascade_iterator('delete', child):
diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py
index 8c77547da..eed50cd6d 100644
--- a/lib/sqlalchemy/orm/dynamic.py
+++ b/lib/sqlalchemy/orm/dynamic.py
@@ -100,7 +100,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl):
         
     def get_history(self, state, passive=False):
         c = self._get_collection_history(state, passive)
-        return (c.added_items, c.unchanged_items, c.deleted_items)
+        return attributes.History(c.added_items, c.unchanged_items, c.deleted_items)
         
     def _get_collection_history(self, state, passive=False):
         if self.key in state.committed_state:
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 279fcd60c..be1be549c 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1148,8 +1148,8 @@ class Mapper(object):
                             params[col._label] = mapper._get_state_attr_by_column(state, col)
                             params[col.key] = params[col._label] + 1
                             for prop in mapper._columntoproperty.values():
-                                (added, unchanged, deleted) = attributes.get_history(state, prop.key, passive=True)
-                                if added:
+                                history = attributes.get_history(state, prop.key, passive=True)
+                                if history.added:
                                     hasdata = True
                         elif mapper.polymorphic_on and mapper.polymorphic_on.shares_lineage(col):
                             pass
@@ -1160,18 +1160,18 @@ class Mapper(object):
                                 continue
 
                             prop = mapper._columntoproperty[col]
-                            (added, unchanged, deleted) = attributes.get_history(state, prop.key, passive=True)
-                            if added:
-                                if isinstance(added[0], sql.ClauseElement):
-                                    value_params[col] = added[0]
+                            history = attributes.get_history(state, prop.key, passive=True)
+                            if history.added:
+                                if isinstance(history.added[0], sql.ClauseElement):
+                                    value_params[col] = history.added[0]
                                 else:
-                                    params[col.key] = prop.get_col_value(col, added[0])
+                                    params[col.key] = prop.get_col_value(col, history.added[0])
                                 if col in pks:
-                                    if deleted:
-                                        params[col._label] = deleted[0]
+                                    if history.deleted:
+                                        params[col._label] = history.deleted[0]
                                     else:
                                         # row switch logic can reach us here
-                                        params[col._label] = added[0]
+                                        params[col._label] = history.added[0]
                                 hasdata = True
                             elif col in pks:
                                 params[col._label] = mapper._get_state_attr_by_column(state, col)
diff --git a/lib/sqlalchemy/orm/sync.py b/lib/sqlalchemy/orm/sync.py
index eca80df25..f338b331f 100644
--- a/lib/sqlalchemy/orm/sync.py
+++ b/lib/sqlalchemy/orm/sync.py
@@ -56,8 +56,8 @@ def source_changes(uowcommit, source, source_mapper, synchronize_pairs):
             prop = source_mapper._get_col_to_prop(l)
         except exc.UnmappedColumnError:
             _raise_col_to_prop(False, source_mapper, l, None, r)
-        (added, unchanged, deleted) = uowcommit.get_attribute_history(source, prop.key, passive=True)
-        if added and deleted:
+        history = uowcommit.get_attribute_history(source, prop.key, passive=True)
+        if history.has_changes():
             return True
     else:
         return False
@@ -68,8 +68,8 @@ def dest_changes(uowcommit, dest, dest_mapper, synchronize_pairs):
             prop = dest_mapper._get_col_to_prop(r)
         except exc.UnmappedColumnError:
             _raise_col_to_prop(True, None, l, dest_mapper, r)
-        (added, unchanged, deleted) = uowcommit.get_attribute_history(dest, prop.key, passive=True)
-        if added and deleted:
+        history = uowcommit.get_attribute_history(dest, prop.key, passive=True)
+        if history.has_changes():
             return True
     else:
         return False
diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index 0992bfe99..778bf0949 100644
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -119,24 +119,20 @@ class UOWTransaction(object):
         # prevents newly loaded objects from being dereferenced during the
         # flush process
         if hashkey in self.attributes:
-            (added, unchanged, deleted, cached_passive) = self.attributes[hashkey]
+            (history, cached_passive) = self.attributes[hashkey]
             # if the cached lookup was "passive" and now we want non-passive, do a non-passive
             # lookup and re-cache
             if cached_passive and not passive:
-                (added, unchanged, deleted) = attributes.get_history(state, key, passive=False)
-                self.attributes[hashkey] = (added, unchanged, deleted, passive)
+                history = attributes.get_history(state, key, passive=False)
+                self.attributes[hashkey] = (history, passive)
         else:
-            (added, unchanged, deleted) = attributes.get_history(state, key, passive=passive)
-            self.attributes[hashkey] = (added, unchanged, deleted, passive)
+            history = attributes.get_history(state, key, passive=passive)
+            self.attributes[hashkey] = (history, passive)
 
-        if added is None or not state.get_impl(key).uses_objects:
-            return (added, unchanged, deleted)
+        if not history or not state.get_impl(key).uses_objects:
+            return history
         else:
-            return (
-                [c is not None and attributes.instance_state(c) or None for c in added],
-                [c is not None and attributes.instance_state(c) or None for c in unchanged],
-                [c is not None and attributes.instance_state(c) or None for c in deleted],
-                )
+            return history.as_state()
 
     def register_object(self, state, isdelete=False, listonly=False, postupdate=False, post_update_cols=None):
         # if object is not in the overall session, do nothing
diff --git a/test/orm/unitofwork.py b/test/orm/unitofwork.py
index 05f4d88f3..975743fb0 100644
--- a/test/orm/unitofwork.py
+++ b/test/orm/unitofwork.py
@@ -694,15 +694,12 @@ class PassiveDeletesTest(_base.MappedTest):
             pass
 
     @testing.resolve_artifact_names
-    def setup_mappers(self):
+    def test_basic(self):
         mapper(MyOtherClass, myothertable)
         mapper(MyClass, mytable, properties={
             'children':relation(MyOtherClass,
                                 passive_deletes=True,
                                 cascade="all")})
-
-    @testing.resolve_artifact_names
-    def test_basic(self):
         session = create_session()
         mc = MyClass()
         mc.children.append(MyOtherClass())
@@ -721,7 +718,33 @@ class PassiveDeletesTest(_base.MappedTest):
 
         assert mytable.count().scalar() == 0
         assert myothertable.count().scalar() == 0
+    
+    @testing.resolve_artifact_names
+    def test_backwards_pd(self):
+        # the unusual scenario where a trigger or something might be deleting
+        # a many-to-one on deletion of the parent row
+        mapper(MyOtherClass, myothertable, properties={
+            'myclass':relation(MyClass, cascade="all, delete", passive_deletes=True)
+        })
+        mapper(MyClass, mytable)
+        
+        session = create_session()
+        mc = MyClass()
+        mco = MyOtherClass()
+        mco.myclass = mc
+        session.add(mco)
+        session.flush()
 
+        assert mytable.count().scalar() == 1
+        assert myothertable.count().scalar() == 1
+        
+        session.expire(mco, ['myclass'])
+        session.delete(mco)
+        session.flush()
+        
+        assert mytable.count().scalar() == 1
+        assert myothertable.count().scalar() == 0
+        
 class ExtraPassiveDeletesTest(_base.MappedTest):
     __requires__ = ('foreign_keys',)