From 92599bfec69afa53ea417b183deb2e45eeb91285 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sun, 13 Jun 2010 16:25:26 -0400
Subject: [PATCH] - 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]

---
 CHANGES                          |  7 +++++
 lib/sqlalchemy/orm/dependency.py | 18 ++++++++-----
 test/orm/test_manytomany.py      | 45 ++++++++++++++++++++++++++------
 3 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/CHANGES b/CHANGES
index efd95605de..714f82bab1 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 b2c3d1fb9d..ba2ae8889c 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 cac5fda781..46d0cc44fd 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')
@@ -128,6 +129,34 @@ class M2MTest(_base.MappedTest):
         [sess.delete(p) for p in p1,p2,p3,p4,p5,p6,p7]
         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
-- 
2.47.2