Commits

Mike Bayer committed f598e5c

- 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

Comments (0)

Files changed (3)

doc/build/changelog/changelog_08.rst

       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

lib/sqlalchemy/orm/dynamic.py

         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
             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):
                         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),

test/orm/test_dynamic.py

         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):
         u1.addresses.remove(a2)
 
         self._assert_history(u1,
-            ([a3], [a1], [a2])
+            ([a3], [a1], [a2]),
+            compare_passive=([a3], [], [a2])
         )
 
     def test_replace_transient(self):
         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])
         )
 
 
 
         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)
 
         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])
+                )