diff options
| -rw-r--r-- | doc/build/changelog/changelog_12.rst | 14 | ||||
| -rw-r--r-- | doc/build/changelog/migration_12.rst | 27 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/collections.py | 3 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/events.py | 8 | ||||
| -rw-r--r-- | test/orm/test_attributes.py | 13 | ||||
| -rw-r--r-- | test/orm/test_collection.py | 6 |
6 files changed, 61 insertions, 10 deletions
diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst index 55050c482..f4fd6a3fe 100644 --- a/doc/build/changelog/changelog_12.rst +++ b/doc/build/changelog/changelog_12.rst @@ -46,6 +46,20 @@ :ref:`change_3891` + .. change:: 3913 + :tags: bug, orm + :tickets: 3913 + + When assigning a collection to an attribute mapped by a relationship, + the previous collection is no longer mutated. Previously, the old + collection would be emptied out in conjunction with the "item remove" + events that fire off; the events now fire off without affecting + the old collection. + + .. seealso:: + + :ref:`change_3913` + .. change:: 3932 :tags: bug, oracle :tickets: 3932 diff --git a/doc/build/changelog/migration_12.rst b/doc/build/changelog/migration_12.rst index d64200d12..da21f11b4 100644 --- a/doc/build/changelog/migration_12.rst +++ b/doc/build/changelog/migration_12.rst @@ -139,6 +139,33 @@ WHERE clause manually may need to be adjusted. :ticket:`3891` +.. _change_3913: + +Previous collection is no longer mutated upon replacement +--------------------------------------------------------- + +The ORM emits events whenever the members of a mapped collection change. +In the case of assigning a collection to an attribute that would replace +the previous collection, a side effect of this was that the collection +being replaced would also be mutated, which is misleading and unnecessary:: + + >>> a1, a2, a3 = Address('a1'), Address('a2'), Address('a3') + >>> user.addresses = [a1, a2] + + >>> previous_collection = user.addresses + + # replace the collection with a new one + >>> user.addresses = [a2, a3] + + >>> previous_collection + [Address('a1'), Address('a2')] + +Above, prior to the change, the ``previous_collection`` would have had the +"a1" member removed, corresponding to the member that's no longer in the +new collection. + +:ticket:`3913` + Key Behavioral Changes - Core ============================= diff --git a/lib/sqlalchemy/orm/collections.py b/lib/sqlalchemy/orm/collections.py index 2bb53e61e..d949dc8a1 100644 --- a/lib/sqlalchemy/orm/collections.py +++ b/lib/sqlalchemy/orm/collections.py @@ -764,9 +764,8 @@ def bulk_replace(values, existing_adapter, new_adapter): appender(member, _sa_initiator=False) if existing_adapter: - remover = existing_adapter.bulk_remover() for member in removals: - remover(member) + existing_adapter.fire_remove_event(member) def prepare_instrumentation(factory): diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py index d2ccec432..ceeb31e9c 100644 --- a/lib/sqlalchemy/orm/events.py +++ b/lib/sqlalchemy/orm/events.py @@ -2124,8 +2124,12 @@ class AttributeEvents(event.Events): u1.addresses = [a2, a3] # <- old collection is disposed - The mechanics of the event will typically include that the given - collection is empty, even if it stored objects while being replaced. + The old collection received will contain its previous contents. + + .. versionchanged:: 1.2 The collection passed to + :meth:`.AttributeEvents.dispose_collection` will now have its + contents before the dispose intact; previously, the collection + would be empty. .. versionadded:: 1.0.0 the :meth:`.AttributeEvents.init_collection` and :meth:`.AttributeEvents.dispose_collection` events supersede diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index d6f3caa02..d3a63c386 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -2687,13 +2687,15 @@ class ListenerTest(fixtures.ORMTest): f1.barlist = [b2] adapter_two = f1.barlist._sa_adapter eq_(canary.init.mock_calls, [ - call(f1, [], adapter_one), + call(f1, [b1], adapter_one), # note the f1.barlist that + # we saved earlier has been mutated + # in place, new as of [ticket:3913] call(f1, [b2], adapter_two), ]) eq_( canary.dispose.mock_calls, [ - call(f1, [], adapter_one) + call(f1, [b1], adapter_one) ] ) @@ -2903,10 +2905,11 @@ class TestUnlink(fixtures.TestBase): coll = a1.bs a1.bs.append(B()) a1.bs = [] - # a bulk replace empties the old collection - assert len(coll) == 0 - coll.append(B()) + # a bulk replace no longer empties the old collection + # as of [ticket:3913] assert len(coll) == 1 + coll.append(B()) + assert len(coll) == 2 def test_pop_existing(self): A, B = self.A, self.B diff --git a/test/orm/test_collection.py b/test/orm/test_collection.py index e3c69a9a6..d059db6e5 100644 --- a/test/orm/test_collection.py +++ b/test/orm/test_collection.py @@ -1551,7 +1551,11 @@ class CollectionsTest(fixtures.ORMTest): bulk1 = [e2] # empty & sever col1 from obj obj.attr = bulk1 - self.assert_(len(col1) == 0) + + # as of [ticket:3913] the old collection + # remains unchanged + self.assert_(len(col1) == 1) + self.assert_(len(canary.data) == 1) self.assert_(obj.attr is not col1) self.assert_(obj.attr is not bulk1) |