- Fixed bug regarding flushes on self-referential

bi-directional many-to-many relationships, where
two objects made to mutually reference each other
in one flush would fail to insert a row for both
sides.  Regression from 0.5. [ticket:1824]

3 files changed, 55 insertions, 15 deletions

diff --git a/CHANGES b/CHANGES
index efd95605d..714f82bab 100644
--- a/CHANGES
+++ b/CHANGES
@@ -5,6 +5,13 @@ CHANGES
 =======
 0.6.2
 =====
+- orm
+  - Fixed bug regarding flushes on self-referential 
+    bi-directional many-to-many relationships, where
+    two objects made to mutually reference each other
+    in one flush would fail to insert a row for both
+    sides.  Regression from 0.5. [ticket:1824]
+    
 - sql
   - The warning emitted by the Unicode and String types
     with convert_unicode=True no longer embeds the actual
diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index b2c3d1fb9..ba2ae8889 100644
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -870,7 +870,7 @@ class ManyToManyDP(DependencyProcessor):
         secondary_update = []
         
         processed = self._get_reversed_processed_set(uowcommit)
-        
+        tmp = set()
         for state in states:
             history = uowcommit.get_attribute_history(
                                     state, 
@@ -890,8 +890,10 @@ class ManyToManyDP(DependencyProcessor):
                                         False, uowcommit)
                     secondary_delete.append(associationrow)
                 
-                if processed is not None:
-                    processed.update((c, state) for c in history.non_added())
+                tmp.update((c, state) for c in history.non_added())
+
+        if processed is not None:
+            processed.update(tmp)
                 
         self._run_crud(uowcommit, secondary_insert, 
                         secondary_update, secondary_delete)
@@ -902,7 +904,8 @@ class ManyToManyDP(DependencyProcessor):
         secondary_update = []
 
         processed = self._get_reversed_processed_set(uowcommit)
-        
+        tmp = set()
+
         for state in states:
             history = uowcommit.get_attribute_history(state, self.key)
             if history:
@@ -928,8 +931,7 @@ class ManyToManyDP(DependencyProcessor):
                                         False, uowcommit)
                     secondary_delete.append(associationrow)
                 
-                if processed is not None:
-                    processed.update((c, state) for c in history.added + history.deleted)
+                tmp.update((c, state) for c in history.added + history.deleted)
                 
             if not self.passive_updates and \
                     self._pks_changed(uowcommit, state):
@@ -954,7 +956,9 @@ class ManyToManyDP(DependencyProcessor):
 
                     secondary_update.append(associationrow)
                     
-
+        if processed is not None:
+            processed.update(tmp)
+            
         self._run_crud(uowcommit, secondary_insert, 
                         secondary_update, secondary_delete)
         
diff --git a/test/orm/test_manytomany.py b/test/orm/test_manytomany.py
index cac5fda78..46d0cc44f 100644
--- a/test/orm/test_manytomany.py
+++ b/test/orm/test_manytomany.py
@@ -82,14 +82,15 @@ class M2MTest(_base.MappedTest):
     def test_circular(self):
         """test a many-to-many relationship from a table to itself."""
 
-        Place.mapper = mapper(Place, place)
-
-        Place.mapper.add_property('places', relationship(
-            Place.mapper, secondary=place_place, primaryjoin=place.c.place_id==place_place.c.pl1_id,
-            secondaryjoin=place.c.place_id==place_place.c.pl2_id,
-            order_by=place_place.c.pl2_id,
-            lazy='select',
-            ))
+        mapper(Place, place, properties={
+            'places': relationship(
+                        Place,
+                        secondary=place_place, 
+                        primaryjoin=place.c.place_id==place_place.c.pl1_id,
+                        secondaryjoin=place.c.place_id==place_place.c.pl2_id,
+                        order_by=place_place.c.pl2_id
+                )
+        })
 
         sess = create_session()
         p1 = Place('place1')
@@ -129,6 +130,34 @@ class M2MTest(_base.MappedTest):
         sess.flush()
 
     @testing.resolve_artifact_names
+    def test_circular_mutation(self):
+        """Test that a mutation in a self-ref m2m of both sides succeeds."""
+
+        mapper(Place, place, properties={
+            'child_places': relationship(
+                        Place,
+                        secondary=place_place, 
+                        primaryjoin=place.c.place_id==place_place.c.pl1_id,
+                        secondaryjoin=place.c.place_id==place_place.c.pl2_id,
+                        order_by=place_place.c.pl2_id,
+                        backref='parent_places'
+                )
+        })
+
+        sess = create_session()
+        p1 = Place('place1')
+        p2 = Place('place2')
+        p2.parent_places = [p1]
+        sess.add_all([p1, p2])
+        p1.parent_places.append(p2)
+        sess.flush()
+        
+        sess.expire_all()
+        assert p1 in p2.parent_places
+        assert p2 in p1.parent_places
+        
+
+    @testing.resolve_artifact_names
     def test_double(self):
         """test that a mapper can have two eager relationships to the same table, via
         two different association tables.  aliases are required."""