From: Mike Bayer Date: Fri, 27 Mar 2009 19:54:10 +0000 (+0000) Subject: - Fixed the "set collection" function on "dynamic" relations X-Git-Tag: rel_0_5_4~52 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=18106b5e5b682aa6aff8ecbab4dae776895c74de;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Fixed the "set collection" function on "dynamic" relations to initiate events correctly. Previously a collection could only be assigned to a pending parent instance, otherwise modified events would not be fired correctly. Set collection is now compatible with merge(), fixes [ticket:1352]. --- diff --git a/CHANGES b/CHANGES index bef324894c..ab09d7902f 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,16 @@ ======= CHANGES ======= +0.5.4 +===== +- orm + - Fixed the "set collection" function on "dynamic" relations + to initiate events correctly. Previously a collection + could only be assigned to a pending parent instance, + otherwise modified events would not be fired correctly. + Set collection is now compatible with merge(), + fixes [ticket:1352]. + 0.5.3 ===== - orm diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index 319364910c..4bc3f58c2e 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -19,7 +19,7 @@ from sqlalchemy.orm import ( ) from sqlalchemy.orm.query import Query from sqlalchemy.orm.util import _state_has_identity, has_identity -from sqlalchemy.orm import attributes +from sqlalchemy.orm import attributes, collections class DynaLoader(strategies.AbstractRelationLoader): def init_class_attribute(self, mapper): @@ -102,12 +102,19 @@ class DynamicAttributeImpl(attributes.AttributeImpl): if initiator is self: return + self._set_iterable(state, value) + + def _set_iterable(self, state, iterable, adapter=None): + collection_history = self._modified_event(state) + new_values = list(iterable) + if _state_has_identity(state): old_collection = list(self.get(state)) else: old_collection = [] - collection_history.replace(old_collection, value) + + collections.bulk_replace(new_values, DynCollectionAdapter(self, state, old_collection), DynCollectionAdapter(self, state, new_values)) def delete(self, *args, **kwargs): raise NotImplementedError() @@ -135,6 +142,28 @@ class DynamicAttributeImpl(attributes.AttributeImpl): if initiator is not self: self.fire_remove_event(state, value, initiator) +class DynCollectionAdapter(object): + """the dynamic analogue to orm.collections.CollectionAdapter""" + + def __init__(self, attr, owner_state, data): + self.attr = attr + self.state = owner_state + self.data = data + + def __iter__(self): + return iter(self.data) + + def append_with_event(self, item, initiator=None): + self.attr.append(self.state, item, initiator) + + def remove_with_event(self, item, initiator=None): + self.attr.remove(self.state, item, initiator) + + def append_without_event(self, item): + pass + + def remove_without_event(self, item): + pass class AppenderMixin(object): query_class = None @@ -239,8 +268,4 @@ class CollectionHistory(object): self.deleted_items = [] self.added_items = [] self.unchanged_items = [] - - def replace(self, olditems, newitems): - self.added_items = newitems - self.deleted_items = olditems diff --git a/test/orm/dynamic.py b/test/orm/dynamic.py index 0a9e578315..f975f762f8 100644 --- a/test/orm/dynamic.py +++ b/test/orm/dynamic.py @@ -3,7 +3,7 @@ import operator from sqlalchemy.orm import dynamic_loader, backref from testlib import testing from testlib.sa import Table, Column, Integer, String, ForeignKey, desc, select, func -from testlib.sa.orm import mapper, relation, create_session, Query +from testlib.sa.orm import mapper, relation, create_session, Query, attributes from testlib.testing import eq_ from testlib.compat import _function_named from orm import _base, _fixtures @@ -152,7 +152,7 @@ class DynamicTest(_fixtures.FixtureTest): assert type(q).__name__ == 'MyQuery' -class FlushTest(_fixtures.FixtureTest): +class SessionTest(_fixtures.FixtureTest): run_inserts = None @testing.resolve_artifact_names @@ -193,9 +193,43 @@ class FlushTest(_fixtures.FixtureTest): (a2.id, u1.id, 'bar') ] + + @testing.resolve_artifact_names + def test_merge(self): + mapper(User, users, properties={ + 'addresses':dynamic_loader(mapper(Address, addresses), order_by=addresses.c.email_address) + }) + sess = create_session() + u1 = User(name='jack') + a1 = Address(email_address='a1') + a2 = Address(email_address='a2') + a3 = Address(email_address='a3') + + u1.addresses.append(a2) + u1.addresses.append(a3) + + sess.add_all([u1, a1]) + sess.flush() + + u1 = User(id=u1.id, name='jack') + u1.addresses.append(a1) + u1.addresses.append(a3) + u1 = sess.merge(u1) + assert attributes.get_history(u1, 'addresses') == ( + [a1], + [a3], + [a2] + ) + + sess.flush() + + eq_( + list(u1.addresses), + [a1, a3] + ) @testing.resolve_artifact_names - def test_basic(self): + def test_flush(self): mapper(User, users, properties={ 'addresses':dynamic_loader(mapper(Address, addresses)) }) @@ -231,6 +265,31 @@ class FlushTest(_fixtures.FixtureTest): assert 'addresses' not in u1.__dict__.keys() u1.addresses = [Address(email_address='test')] assert 'addresses' in dir(u1) + + @testing.resolve_artifact_names + def test_collection_set(self): + mapper(User, users, properties={ + 'addresses':dynamic_loader(mapper(Address, addresses), order_by=addresses.c.email_address) + }) + sess = create_session(autoflush=True, autocommit=False) + u1 = User(name='jack') + a1 = Address(email_address='a1') + a2 = Address(email_address='a2') + a3 = Address(email_address='a3') + a4 = Address(email_address='a4') + + sess.add(u1) + u1.addresses = [a1, a3] + assert list(u1.addresses) == [a1, a3] + u1.addresses = [a1, a2, a4] + assert list(u1.addresses) == [a1, a2, a4] + u1.addresses = [a2, a3] + assert list(u1.addresses) == [a2, a3] + u1.addresses = [] + assert list(u1.addresses) == [] + + + @testing.resolve_artifact_names def test_rollback(self): @@ -392,7 +451,7 @@ def create_backref_test(autoflush, saveuser): test_backref = _function_named( test_backref, "test%s%s" % ((autoflush and "_autoflush" or ""), (saveuser and "_saveuser" or "_savead"))) - setattr(FlushTest, test_backref.__name__, test_backref) + setattr(SessionTest, test_backref.__name__, test_backref) for autoflush in (False, True): for saveuser in (False, True):