From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sat, 22 Dec 2012 15:36:55 +0000 (-0500)
Subject: - revert the full iteration of the collection for a passive history event; that's
X-Git-Tag: rel_0_8_0~38^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=efbbe88705eb19d68b587aae6dfb60cfe4356edb;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- revert the full iteration of the collection for a passive history event; that's
the wrong behavior, and for the original #2637 issue we will have to fix the
association proxy, which is #2642
---

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 4eb7fc7122..0cb8da9a3c 100644
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -14,8 +14,7 @@
       Fixes to the "dynamic" loader on :func:`.relationship`, includes
       that backrefs will work properly even when autoflush is disabled,
       history events are more accurate in scenarios where multiple add/remove
-      of the same object occurs, as can often be the case in conjunction
-      with the association proxy.
+      of the same object occurs.
 
     .. change::
         :tags: sqlite, bug
diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py
index 28bddd6130..c193f0e2a4 100644
--- a/lib/sqlalchemy/orm/dynamic.py
+++ b/lib/sqlalchemy/orm/dynamic.py
@@ -180,7 +180,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl):
         else:
             c = CollectionHistory(self, state)
 
-        if state.has_identity:
+        if state.has_identity and (passive & attributes.INIT_OK):
             return CollectionHistory(self, state, apply_to=c)
         else:
             return c
@@ -314,10 +314,12 @@ class CollectionHistory(object):
             self.unchanged_items = util.OrderedIdentitySet(coll)
             self.added_items = apply_to.added_items
             self.deleted_items = apply_to.deleted_items
+            self._reconcile_collection = True
         else:
             self.deleted_items = util.OrderedIdentitySet()
             self.added_items = util.OrderedIdentitySet()
             self.unchanged_items = util.OrderedIdentitySet()
+            self._reconcile_collection = False
 
     @property
     def added_plus_unchanged(self):
@@ -329,10 +331,14 @@ class CollectionHistory(object):
                         self.unchanged_items).union(self.deleted_items))
 
     def as_history(self):
-        added = self.added_items.difference(self.unchanged_items)
-        deleted = self.deleted_items.intersection(self.unchanged_items)
-        unchanged = self.unchanged_items.difference(deleted)
-
+        if self._reconcile_collection:
+            added = self.added_items.difference(self.unchanged_items)
+            deleted = self.deleted_items.intersection(self.unchanged_items)
+            unchanged = self.unchanged_items.difference(deleted)
+        else:
+            added, unchanged, deleted = self.added_items,\
+                                            self.unchanged_items,\
+                                            self.deleted_items
         return attributes.History(
                     list(added),
                     list(unchanged),
diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py
index 0eef8f5a53..076ef81262 100644
--- a/test/orm/test_dynamic.py
+++ b/test/orm/test_dynamic.py
@@ -656,16 +656,19 @@ class HistoryTest(_DynamicFixture, _fixtures.FixtureTest):
         s.flush()
         return u1, a1, s
 
-    def _assert_history(self, obj, compare):
+    def _assert_history(self, obj, compare, compare_passive=None):
         eq_(
             attributes.get_history(obj, 'addresses'),
             compare
         )
 
+        if compare_passive is None:
+            compare_passive = compare
+
         eq_(
             attributes.get_history(obj, 'addresses',
                         attributes.LOAD_AGAINST_COMMITTED),
-            compare
+            compare_passive
         )
 
     def test_append_transient(self):
@@ -719,7 +722,8 @@ class HistoryTest(_DynamicFixture, _fixtures.FixtureTest):
         u1.addresses.remove(a2)
 
         self._assert_history(u1,
-            ([a3], [a1], [a2])
+            ([a3], [a1], [a2]),
+            compare_passive=([a3], [], [a2])
         )
 
     def test_replace_transient(self):
@@ -767,7 +771,8 @@ class HistoryTest(_DynamicFixture, _fixtures.FixtureTest):
         u1.addresses = [a2, a3, a4, a5]
 
         self._assert_history(u1,
-            ([a3, a4, a5], [a2], [a1])
+            ([a3, a4, a5], [a2], [a1]),
+            compare_passive=([a3, a4, a5], [], [a1])
         )
 
 
@@ -778,7 +783,10 @@ class HistoryTest(_DynamicFixture, _fixtures.FixtureTest):
 
         u1.addresses.append(a1)
 
-        self._assert_history(u1, ([], [a1], []))
+        self._assert_history(u1,
+            ([], [a1], []),
+            compare_passive=([a1], [], [])
+        )
 
     def test_persistent_but_readded_autoflush(self):
         u1, a1, s = self._persistent_fixture(autoflush=True)
@@ -787,11 +795,17 @@ class HistoryTest(_DynamicFixture, _fixtures.FixtureTest):
 
         u1.addresses.append(a1)
 
-        self._assert_history(u1, ([], [a1], []))
+        self._assert_history(u1,
+            ([], [a1], []),
+            compare_passive=([a1], [], [])
+        )
 
     def test_missing_but_removed_noflush(self):
         u1, a1, s = self._persistent_fixture(autoflush=False)
 
         u1.addresses.remove(a1)
 
-        self._assert_history(u1, ([], [], []))
+        self._assert_history(u1,
+                    ([], [], []),
+                    compare_passive=([], [], [a1])
+                )