diff options
| -rw-r--r-- | CHANGES | 12 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/session.py | 7 | ||||
| -rw-r--r-- | test/orm/merge.py | 106 |
3 files changed, 119 insertions, 6 deletions
@@ -12,7 +12,17 @@ CHANGES - clarified the error message which occurs when you try to update() an instance with the same identity key as an instance already present in the session. - + + - some clarifications and fixes to merge(instance, dont_load=True). + fixed bug where lazy loaders were getting disabled on returned instances. + Also, we currently do not support merging an instance which has uncommitted + changes on it, in the case that dont_load=True is used....this will + now raise an error. This is due to complexities in merging the + "committed state" of the given instance to correctly correspond to the + newly copied instance. Since the use case for dont_load=True is + caching, the given instances shouldn't have any uncommitted changes on them + anyway. + 0.4.1 ----- diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 100727520..b04c62c7b 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -881,10 +881,13 @@ class Session(object): if key in self.identity_map: merged = self.identity_map[key] elif dont_load: + if object._state.modified: + raise exceptions.InvalidRequestError("merge() with dont_load=True option does not support objects marked as 'dirty'. flush() all changes on mapped instances before merging with dont_load=True.") + merged = attribute_manager.new_instance(mapper.class_) merged._instance_key = key + merged._entity_name = entity_name self._update_impl(merged, entity_name=mapper.entity_name) - merged._state.committed_state = object._state.committed_state.copy() else: merged = self.get(mapper.class_, key[1]) if merged is None: @@ -894,6 +897,8 @@ class Session(object): prop.merge(self, object, merged, dont_load, _recursive) if key is None: self.save(merged, entity_name=mapper.entity_name) + elif dont_load: + merged._state.commit_all() return merged def identity_key(cls, *args, **kwargs): diff --git a/test/orm/merge.py b/test/orm/merge.py index ed55e1f49..f6df8e9ad 100644 --- a/test/orm/merge.py +++ b/test/orm/merge.py @@ -1,6 +1,8 @@ import testbase from sqlalchemy import * +from sqlalchemy import exceptions from sqlalchemy.orm import * +from sqlalchemy.orm import mapperlib from testlib import * from testlib.tables import * import testlib.tables as tables @@ -122,18 +124,23 @@ class MergeTest(AssertMixin): def go(): sess5.flush() # no changes; therefore flush should do nothing + # but also, dont_load wipes out any difference in committed state, + # so no flush at all self.assert_sql_count(testbase.db, go, 0) - # pre merge change - u.user_name='fred3' sess4 = create_session() u = sess4.merge(u, dont_load=True) # post merge change u.addresses[1].email_address='afafds' def go(): sess4.flush() - # changes still flush - self.assert_sql_count(testbase.db, go, 2) + # afafds change flushes + self.assert_sql_count(testbase.db, go, 1) + + sess5 = create_session() + u2 = sess5.query(User).get(u.user_id) + assert u2.user_name == 'fred2' + assert u2.addresses[1].email_address == 'afafds' def test_saved_cascade_2(self): """tests a more involved merge""" @@ -197,6 +204,97 @@ class MergeTest(AssertMixin): u2.address.email_address = 'hoho@lalala.com' u3 = sess.merge(u2) + + 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 + which themselves are to be merged. This committed_state would instead need to be piecemeal + 'converted' to represent the correct objects. + However, at the moment I'd rather not support this use case; if you are merging with dont_load=True, + you're typically dealing with caching and the merged objects shouldnt be "dirty". + """ + mapper(User, users, properties={ + 'addresses':relation(mapper(Address, addresses),uselist = True) + }) + sess = create_session() + u = User() + u.user_id = 7 + u.user_name = "fred" + a1 = Address() + a1.email_address='foo@bar.com' + u.addresses.append(a1) + + sess.save(u) + sess.flush() + + sess2 = create_session() + u2 = sess2.query(User).options(eagerload('addresses')).get(7) + + sess3 = create_session() + u3 = sess3.merge(u2, dont_load=True) + def go(): + sess3.flush() + self.assert_sql_count(testbase.db, go, 0) + + def test_noload_disallows_dirty(self): + """noload doesnt support 'dirty' objects right now (see test_noload_with_eager()). + Therefore lets assert it.""" + + mapper(User, users) + sess = create_session() + u = User() + u.user_id = 7 + u.user_name = "fred" + sess.save(u) + sess.flush() + + u.user_name = 'ed' + sess2 = create_session() + try: + sess2.merge(u, dont_load=True) + assert False + except exceptions.InvalidRequestError, e: + assert "merge() with dont_load=True option does not support objects marked as 'dirty'. flush() all changes on mapped instances before merging with dont_load=True." in str(e) + + u2 = sess2.query(User).options(eagerload('addresses')).get(7) + + sess3 = create_session() + u3 = sess3.merge(u2, dont_load=True) + def go(): + sess3.flush() + self.assert_sql_count(testbase.db, go, 0) + + def test_noload_sets_entityname(self): + """test that a noload-merged entity has entity_name set, has_mapper() passes, and lazyloads work""" + mapper(User, users, properties={ + 'addresses':relation(mapper(Address, addresses),uselist = True) + }) + sess = create_session() + u = User() + u.user_id = 7 + u.user_name = "fred" + a1 = Address() + a1.email_address='foo@bar.com' + u.addresses.append(a1) + + sess.save(u) + sess.flush() + sess.clear() + + # reload 'u' such that its addresses list hasn't loaded + u = sess.query(User).get(7) + + sess2 = create_session() + u2 = sess2.merge(u, dont_load=True) + # assert merged instance has a mapper and lazy load proceeds + assert hasattr(u2, '_entity_name') + assert mapperlib.has_mapper(u2) + def go(): + assert u2.addresses != [] + assert len(u2.addresses) == 1 + self.assert_sql_count(testbase.db, go, 1) + + if __name__ == "__main__": testbase.main() |