From: Mike Bayer Date: Wed, 30 Jan 2008 17:35:20 +0000 (+0000) Subject: - next release will be 0.4.3 X-Git-Tag: rel_0_4_3~64 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d2e4c52b9f7c7484a6c6722446971b8980472e87;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - next release will be 0.4.3 - fixed merge() collection-doubling bug when merging transient entities with backref'ed collections. [ticket:961] - merge(dont_load=True) does not accept transient entities, this is in continuation with the fact that merge(dont_load=True) does not accept any "dirty" objects either. --- diff --git a/CHANGES b/CHANGES index 2359c35083..1175065fd0 100644 --- a/CHANGES +++ b/CHANGES @@ -2,7 +2,7 @@ CHANGES ======= -0.4.2p4 +0.4.3 ------ - sql - Added "ilike()" operator to column operations. Compiles @@ -32,6 +32,25 @@ CHANGES works fully for all embedded selectables. - orm + - Every Session.begin() must now be accompanied by a + corresponding commit() or rollback() unless the session + is closed with Session.close(). This also includes + the begin() which is implicit to a session created + with transactional=True. The biggest change + introduced here is that when a Session created with + transactional=True raises an exception during flush(), + you must call Session.rollback() or Session.close() in + order for that Session to continue after an exception. + + - fixed merge() collection-doubling bug when merging + transient entities with backref'ed collections. + [ticket:961] + + - merge(dont_load=True) does not accept transient + entities, this is in continuation with the fact that + merge(dont_load=True) does not accept any "dirty" + objects either. + - added standalone "query" class attribute generated by a scoped_session. This provides MyClass.query without using Session.mapper. Use via: @@ -102,37 +121,34 @@ CHANGES column references a server-side-generated, non-primary-key column. [ticket:954] - - Every begin() must now be accompanied by a corresponding - commit() or rollback(), including the implicit begin() - in transactional sessions. - - - Fixed bug with session transaction management: parent - transactions weren't started on the connection when - adding a connection to a nested transaction. + - Additional Session transaction fixes/changes: + - Fixed bug with session transaction management: parent + transactions weren't started on the connection when + adding a connection to a nested transaction. - - session.transaction now always refers to the innermost - active transaction, even when commit/rollback are called - directly on the session transaction object. + - session.transaction now always refers to the innermost + active transaction, even when commit/rollback are + called directly on the session transaction object. - - Two-phase transactions can now be prepared. + - Two-phase transactions can now be prepared. - - When preparing a two-phase transaction fails on one - connection, all the connections are rolled back. + - When preparing a two-phase transaction fails on one + connection, all the connections are rolled back. - - session.close() didn't close all transactions when - nested transactions were used. + - session.close() didn't close all transactions when + nested transactions were used. - - rollback() previously erroneously set the current - transaction directly to the parent of the transaction - that could be rolled back to. Now it rolls back the next - transaction up that can handle it, but sets the current - transaction to it's parent and inactivates the - transactions in between. Inactive transactions can only - be rolled back or closed, any other call results in an - error. + - rollback() previously erroneously set the current + transaction directly to the parent of the transaction + that could be rolled back to. Now it rolls back the + next transaction up that can handle it, but sets the + current transaction to it's parent and inactivates the + transactions in between. Inactive transactions can + only be rolled back or closed, any other call results + in an error. - - autoflush for commit() wasn't flushing for simple - subtransactions. + - autoflush for commit() wasn't flushing for simple + subtransactions. - Miscellaneous tickets: [ticket:940] @@ -158,8 +174,8 @@ CHANGES - Changed ext.activemapper to use a non-transactional session for the objectstore. - - Fixed output order of "['a'] + obj.proxied" binary operation on - association-proxied lists. + - Fixed output order of "['a'] + obj.proxied" binary + operation on association-proxied lists. 0.4.2p3 ------ diff --git a/VERSION b/VERSION index b173bddfbb..17b2ccd9bf 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.4.2p4 +0.4.3 diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 1a17e8599d..7071652154 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -388,8 +388,8 @@ class ManyToManyDP(DependencyProcessor): secondary_insert = [] secondary_update = [] - if hasattr(self.prop, 'reverse_property'): - reverse_dep = getattr(self.prop.reverse_property, '_dependency_processor', None) + if self.prop._reverse_property: + reverse_dep = getattr(self.prop._reverse_property, '_dependency_processor', None) else: reverse_dep = None diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index ca430378b8..a7932bfc55 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -212,7 +212,8 @@ class PropertyLoader(StrategizedProperty): self.comparator = PropertyLoader.Comparator(self) self.join_depth = join_depth self.strategy_class = strategy_class - + self._reverse_property = None + if cascade is not None: self.cascade = CascadeOptions(cascade) else: @@ -349,17 +350,22 @@ class PropertyLoader(StrategizedProperty): return str(self.parent.class_.__name__) + "." + self.key + " (" + str(self.mapper.class_.__name__) + ")" def merge(self, session, source, dest, dont_load, _recursive): + if not dont_load and self._reverse_property and (source, self._reverse_property) in _recursive: + return + if not "merge" in self.cascade: # TODO: lazy callable should merge to the new instance dest._state.expire_attributes([self.key]) return + instances = attributes.get_as_list(source._state, self.key, passive=True) if not instances: return + if self.uselist: - # sets a blank collection according to the correct list class dest_list = attributes.init_collection(dest, self.key) for current in instances: + _recursive[(current, self)] = True obj = session.merge(current, entity_name=self.mapper.entity_name, dont_load=dont_load, _recursive=_recursive) if obj is not None: if dont_load: @@ -369,6 +375,7 @@ class PropertyLoader(StrategizedProperty): else: current = instances[0] if current is not None: + _recursive[(current, self)] = True obj = session.merge(current, entity_name=self.mapper.entity_name, dont_load=dont_load, _recursive=_recursive) if obj is not None: if dont_load: @@ -781,8 +788,8 @@ class BackRef(object): mapper._compile_property(self.key, relation); - prop.reverse_property = mapper._get_property(self.key) - mapper._get_property(self.key).reverse_property = prop + prop._reverse_property = mapper._get_property(self.key) + mapper._get_property(self.key)._reverse_property = prop else: raise exceptions.ArgumentError("Error creating backref '%s' on relation '%s': property of that name exists on mapper '%s'" % (self.key, prop, mapper)) diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 03471df341..b689b24411 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -949,7 +949,8 @@ class Session(object): """ if _recursive is None: - _recursive = {} #TODO: this should be an IdentityDict + _recursive = {} # TODO: this should be an IdentityDict for instances, but will need a separate + # dict for PropertyLoader tuples if entity_name is not None: mapper = _class_mapper(instance.__class__, entity_name=entity_name) else: @@ -959,6 +960,8 @@ class Session(object): key = getattr(instance, '_instance_key', None) if key is None: + if dont_load: + raise exceptions.InvalidRequestError("merge() with dont_load=True option does not support objects transient (i.e. unpersisted) objects. flush() all changes on mapped instances before merging with dont_load=True.") merged = attributes.new_instance(mapper.class_) else: if key in self.identity_map: diff --git a/test/orm/merge.py b/test/orm/merge.py index 9fa93ffb2c..cbaf752d1f 100644 --- a/test/orm/merge.py +++ b/test/orm/merge.py @@ -4,6 +4,7 @@ from sqlalchemy import exceptions from sqlalchemy.orm import * from sqlalchemy.orm import mapperlib from testlib import * +from testlib import fixtures from testlib.tables import * import testlib.tables as tables @@ -37,53 +38,67 @@ class MergeTest(AssertMixin): assert u2.user_name == 'fred' def test_unsaved_cascade(self): - """test merge of a transient entity with two child transient entities.""" + """test merge of a transient entity with two child transient entities, with a bidirectional relation.""" + + class User(fixtures.Base): + pass + class Address(fixtures.Base): + pass + mapper(User, users, properties={ - 'addresses':relation(mapper(Address, addresses), cascade="all") + 'addresses':relation(mapper(Address, addresses), cascade="all", backref="user") }) sess = create_session() - u = User() - u.user_id = 7 - u.user_name = "fred" - a1 = Address() - a1.email_address='foo@bar.com' - a2 = Address() - a2.email_address = 'hoho@la.com' + u = User(user_id=7, user_name='fred') + a1 = Address(email_address='foo@bar.com') + a2 = Address(email_address='hoho@bar.com') u.addresses.append(a1) u.addresses.append(a2) u2 = sess.merge(u) - self.assert_result([u], User, {'user_id':7, 'user_name':'fred', 'addresses':(Address, [{'email_address':'foo@bar.com'}, {'email_address':'hoho@la.com'}])}) - self.assert_result([u2], User, {'user_id':7, 'user_name':'fred', 'addresses':(Address, [{'email_address':'foo@bar.com'}, {'email_address':'hoho@la.com'}])}) + self.assertEquals(u, User(user_id=7, user_name='fred', addresses=[Address(email_address='foo@bar.com'), Address(email_address='hoho@bar.com')])) + self.assertEquals(u2, User(user_id=7, user_name='fred', addresses=[Address(email_address='foo@bar.com'), Address(email_address='hoho@bar.com')])) sess.flush() sess.clear() u2 = sess.query(User).get(7) - self.assert_result([u2], User, {'user_id':7, 'user_name':'fred', 'addresses':(Address, [{'email_address':'foo@bar.com'}, {'email_address':'hoho@la.com'}])}) + self.assertEquals(u2, User(user_id=7, user_name='fred', addresses=[Address(email_address='foo@bar.com'), Address(email_address='hoho@bar.com')])) + def test_transient_dontload(self): + mapper(User, users) + + sess = create_session() + u = User() + try: + u2 = sess.merge(u, dont_load=True) + assert False + except exceptions.InvalidRequestError, err: + assert str(err) == "merge() with dont_load=True option does not support objects transient (i.e. unpersisted) objects. flush() all changes on mapped instances before merging with dont_load=True." + def test_saved_cascade(self): """test merge of a persistent entity with two child persistent entities.""" + + class User(fixtures.Base): + pass + class Address(fixtures.Base): + pass + mapper(User, users, properties={ 'addresses':relation(mapper(Address, addresses), backref='user') }) sess = create_session() # set up data and save - u = User() - u.user_id = 7 - u.user_name = "fred" - a1 = Address() - a1.email_address='foo@bar.com' - a2 = Address() - a2.email_address = 'hoho@la.com' - u.addresses.append(a1) - u.addresses.append(a2) + u = User(user_id=7, user_name='fred', addresses=[ + Address(email_address='foo@bar.com'), + Address(email_address = 'hoho@la.com') + ]) sess.save(u) sess.flush() # assert data was saved sess2 = create_session() u2 = sess2.query(User).get(7) - self.assert_result([u2], User, {'user_id':7, 'user_name':'fred', 'addresses':(Address, [{'email_address':'foo@bar.com'}, {'email_address':'hoho@la.com'}])}) + self.assertEquals(u2, User(user_id=7, user_name='fred', addresses=[Address(email_address='foo@bar.com'), Address(email_address='hoho@la.com')])) # make local changes to data u.user_name = 'fred2' @@ -92,16 +107,17 @@ class MergeTest(AssertMixin): # new session, merge modified data into session sess3 = create_session() u3 = sess3.merge(u) - # insure local changes are pending - self.assert_result([u3], User, {'user_id':7, 'user_name':'fred2', 'addresses':(Address, [{'email_address':'foo@bar.com'}, {'email_address':'hoho@lalala.com'}])}) + # ensure local changes are pending + self.assertEquals(u3, User(user_id=7, user_name='fred2', addresses=[Address(email_address='foo@bar.com'), Address(email_address='hoho@lalala.com')])) + # save merged data sess3.flush() # assert modified/merged data was saved sess.clear() u = sess.query(User).get(7) - self.assert_result([u], User, {'user_id':7, 'user_name':'fred2', 'addresses':(Address, [{'email_address':'foo@bar.com'}, {'email_address':'hoho@lalala.com'}])}) + self.assertEquals(u, User(user_id=7, user_name='fred2', addresses=[Address(email_address='foo@bar.com'), Address(email_address='hoho@lalala.com')])) # merge persistent object into another session sess4 = create_session() @@ -143,7 +159,7 @@ class MergeTest(AssertMixin): assert u2.addresses[1].email_address == 'afafds' def test_saved_cascade_2(self): - """tests a more involved merge""" + mapper(Order, orders, properties={ 'items':relation(mapper(Item, orderitems)) }) @@ -204,7 +220,40 @@ class MergeTest(AssertMixin): u2.address.email_address = 'hoho@lalala.com' u3 = sess.merge(u2) + + def test_dontload_with_backrefs(self): + """test that dontload populates relations in both directions without requiring a load""" + + class User(fixtures.Base): + pass + class Address(fixtures.Base): + pass + mapper(User, users, properties={ + 'addresses':relation(mapper(Address, addresses), backref='user') + }) + + u = User(user_id=7, user_name='fred', addresses=[Address(email_address='ad1'), Address(email_address='ad2')]) + sess = create_session() + sess.save(u) + sess.flush() + sess.close() + assert 'user' in u.addresses[1].__dict__ + + sess = create_session() + u2 = sess.merge(u, dont_load=True) + assert 'user' in u2.addresses[1].__dict__ + self.assertEquals(u2.addresses[1].user, User(user_id=7, user_name='fred')) + + sess.expire(u2.addresses[1], ['user']) + assert 'user' not in u2.addresses[1].__dict__ + sess.close() + sess = create_session() + u = sess.merge(u2, dont_load=True) + assert 'user' not in u.addresses[1].__dict__ + self.assertEquals(u.addresses[1].user, User(user_id=7, user_name='fred')) + + def test_noload_with_eager(self): """this test illustrates that with noload=True, we can't just copy the committed_state of the merged instance over; since it references collection objects @@ -215,7 +264,7 @@ class MergeTest(AssertMixin): """ mapper(User, users, properties={ - 'addresses':relation(mapper(Address, addresses),uselist = True) + 'addresses':relation(mapper(Address, addresses)) }) sess = create_session() u = User()