From: Mike Bayer Date: Sun, 13 Feb 2011 03:20:47 +0000 (-0500) Subject: - Fixed bug whereby Session.merge() would call the X-Git-Tag: rel_0_7b2~1^2~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=838d78af711ccda918a702e01b5630b787cec453;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Fixed bug whereby Session.merge() would call the load() event with one too few arguments. [ticket:2053] - Added logic which prevents the generation of events from a MapperExtension or SessionExtension from generating do-nothing events for all the methods not overridden. [ticket:2052] --- diff --git a/CHANGES b/CHANGES index 5899b500f1..b9d055e2ee 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,18 @@ ======= CHANGES ======= +0.7.0b2 +======== +- orm + - Fixed bug whereby Session.merge() would call the + load() event with one too few arguments. + [ticket:2053] + + - Added logic which prevents the generation of + events from a MapperExtension or SessionExtension + from generating do-nothing events for all the methods + not overridden. [ticket:2052] + 0.7.0b1 ======= - Detailed descriptions of each change below are diff --git a/lib/sqlalchemy/orm/deprecated_interfaces.py b/lib/sqlalchemy/orm/deprecated_interfaces.py index 8cdde22828..1356644690 100644 --- a/lib/sqlalchemy/orm/deprecated_interfaces.py +++ b/lib/sqlalchemy/orm/deprecated_interfaces.py @@ -83,10 +83,7 @@ class MapperExtension(object): me_meth = getattr(MapperExtension, meth) ls_meth = getattr(listener, meth) - # TODO: comparing self.methods to cls.method, - # this comparison is probably moot - - if me_meth is not ls_meth: + if not util.methods_equivalent(me_meth, ls_meth): if meth == 'reconstruct_instance': def go(ls_meth): def reconstruct(instance, ctx): @@ -401,16 +398,23 @@ class SessionExtension(object): @classmethod def _adapt_listener(cls, self, listener): - event.listen(self, 'before_commit', listener.before_commit) - event.listen(self, 'after_commit', listener.after_commit) - event.listen(self, 'after_rollback', listener.after_rollback) - event.listen(self, 'before_flush', listener.before_flush) - event.listen(self, 'after_flush', listener.after_flush) - event.listen(self, 'after_flush_postexec', listener.after_flush_postexec) - event.listen(self, 'after_begin', listener.after_begin) - event.listen(self, 'after_attach', listener.after_attach) - event.listen(self, 'after_bulk_update', listener.after_bulk_update) - event.listen(self, 'after_bulk_delete', listener.after_bulk_delete) + for meth in [ + 'before_commit', + 'after_commit', + 'after_rollback', + 'before_flush', + 'after_flush', + 'after_flush_postexec', + 'after_begin', + 'after_attach', + 'after_bulk_update', + 'after_bulk_delete', + ]: + me_meth = getattr(SessionExtension, meth) + ls_meth = getattr(listener, meth) + + if not util.methods_equivalent(me_meth, ls_meth): + event.listen(self, meth, getattr(listener, meth)) def before_commit(self, session): """Execute right before commit is called. diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py index 1e53a02794..de2147d368 100644 --- a/lib/sqlalchemy/orm/events.py +++ b/lib/sqlalchemy/orm/events.py @@ -181,7 +181,9 @@ class InstanceEvents(event.Events): instead be the :class:`.InstanceState` state-management object associated with the instance. :param context: the :class:`.QueryContext` corresponding to the - current :class:`.Query` in progress. + current :class:`.Query` in progress. This argument may be + ``None`` if the load does not correspond to a :class:`.Query`, + such as during :meth:`.Session.merge`. """ diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 53f29ffeb8..74e8a766fe 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -1318,7 +1318,7 @@ class Session(object): merged_state.commit_all(merged_dict, self.identity_map) if new_instance: - merged_state.manager.dispatch.load(merged_state) + merged_state.manager.dispatch.load(merged_state, None) return merged @classmethod diff --git a/lib/sqlalchemy/util/__init__.py b/lib/sqlalchemy/util/__init__.py index f4861e5f4b..015cc43db8 100644 --- a/lib/sqlalchemy/util/__init__.py +++ b/lib/sqlalchemy/util/__init__.py @@ -25,7 +25,7 @@ from langhelpers import iterate_attributes, class_hierarchy, \ monkeypatch_proxied_specials, asbool, bool_or_str, coerce_kw_type,\ duck_type_collection, assert_arg_type, symbol, dictlike_iteritems,\ classproperty, set_creation_order, warn_exception, warn, NoneType,\ - constructor_copy + constructor_copy, methods_equivalent from deprecations import warn_deprecated, warn_pending_deprecation, \ deprecated, pending_deprecation diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py index 4088e85cb4..7a7713d1e2 100644 --- a/lib/sqlalchemy/util/langhelpers.py +++ b/lib/sqlalchemy/util/langhelpers.py @@ -323,6 +323,16 @@ def monkeypatch_proxied_specials(into_cls, from_cls, skip=None, only=None, pass setattr(into_cls, method, env[method]) + +def methods_equivalent(meth1, meth2): + """Return True if the two methods are the same implementation.""" + + # Py3k + #return getattr(meth1, '__func__', meth1) is getattr(meth2, '__func__', meth2) + # Py2K + return getattr(meth1, 'im_func', meth1) is getattr(meth2, 'im_func', meth2) + # end Py2K + def as_interface(obj, cls=None, methods=None, required=None): """Ensure basic interface compliance for an instance or dict of callables. diff --git a/test/orm/test_events.py b/test/orm/test_events.py index 0ab1c17e3d..47c53628b9 100644 --- a/test/orm/test_events.py +++ b/test/orm/test_events.py @@ -123,6 +123,27 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest): 'populate_instance', 'load', 'append_result', 'before_update', 'after_update', 'before_delete', 'after_delete']) + @testing.resolve_artifact_names + def test_merge(self): + mapper(User, users) + + canary =[] + def load(obj, ctx): + canary.append('load') + event.listen(mapper, 'load', load) + + s = Session() + u = User(name='u1') + s.add(u) + s.commit() + s = Session() + u2 = s.merge(u) + s = Session() + u2 = s.merge(User(name='u2')) + s.commit() + s.query(User).first() + eq_(canary,['load', 'load', 'load']) + @testing.resolve_artifact_names def test_inheritance(self): class AdminUser(User): @@ -864,6 +885,19 @@ class MapperExtensionTest(_fixtures.FixtureTest): sess.expunge_all() assert sess.query(User).first() + @testing.resolve_artifact_names + def test_unnecessary_methods_not_evented(self): + class MyExtension(sa.orm.MapperExtension): + def before_insert(self, mapper, connection, instance): + pass + + class Foo(object): + pass + m = mapper(Foo, users, extension=MyExtension()) + assert not m.class_manager.dispatch.load + assert not m.dispatch.before_update + assert len(m.dispatch.before_insert) == 1 + class AttributeExtensionTest(_base.MappedTest): @classmethod @@ -1029,3 +1063,13 @@ class SessionExtensionTest(_fixtures.FixtureTest): 'before_commit_two', ] + @testing.resolve_artifact_names + def test_unnecessary_methods_not_evented(self): + class MyExtension(sa.orm.session.SessionExtension): + def before_commit(self, session): + pass + + s = Session(extension=MyExtension()) + assert not s.dispatch.after_commit + assert len(s.dispatch.before_commit) == 1 +