From ab738c21aa540cebf3c2f839e177146fc4b43672 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 9 Feb 2014 16:47:20 -0500 Subject: [PATCH] - Fixed an 0.9 regression where ORM instance or mapper events applied to a base class such as a declarative base with the propagate=True flag would fail to apply to existing mapped classes which also used inheritance due to an assertion. Addtionally, repaired an attribute error which could occur during removal of such an event, depending on how it was first assigned. [ticket:2949] --- doc/build/changelog/changelog_09.rst | 11 ++++++ lib/sqlalchemy/orm/events.py | 30 ++++++--------- test/orm/test_events.py | 57 ++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 18 deletions(-) diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index 1db54d85cc..62db3e6997 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -14,6 +14,17 @@ .. changelog:: :version: 0.9.3 + .. change:: + :tags: bug, orm + :tickets: 2949 + + Fixed an 0.9 regression where ORM instance or mapper events applied + to a base class such as a declarative base with the propagate=True + flag would fail to apply to existing mapped classes which also + used inheritance due to an assertion. Addtionally, repaired an + attribute error which could occur during removal of such an event, + depending on how it was first assigned. + .. change:: :tags: bug, ext diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py index a09154dd00..b2c356f246 100644 --- a/lib/sqlalchemy/orm/events.py +++ b/lib/sqlalchemy/orm/events.py @@ -192,12 +192,9 @@ class InstanceEvents(event.Events): event_key.dispatch_target, event_key.identifier, event_key.fn if not raw: - orig_fn = fn - def wrap(state, *arg, **kw): - return orig_fn(state.obj(), *arg, **kw) - fn = wrap - event_key = event_key.with_wrapper(fn) + return fn(state.obj(), *arg, **kw) + event_key = event_key.with_wrapper(wrap) event_key.base_listen(propagate=propagate) @@ -369,15 +366,18 @@ class _EventsHold(event.RefCollection): stack.extend(subclass.__subclasses__()) subject = target.resolve(subclass) if subject is not None: + # we are already going through __subclasses__() + # so leave generic propagate flag False event_key.with_dispatch_target(subject).\ - listen(raw=raw, propagate=propagate) + listen(raw=raw, propagate=False) def remove(self, event_key): target, identifier, fn = \ event_key.dispatch_target, event_key.identifier, event_key.fn - collection = target.all_holds[target.class_] - del collection[event_key._key] + if isinstance(target, _EventsHold): + collection = target.all_holds[target.class_] + del collection[event_key._key] @classmethod def populate(cls, class_, subject): @@ -517,18 +517,15 @@ class MapperEvents(event.Events): except ValueError: target_index = None - wrapped_fn = fn - def wrap(*arg, **kw): if not raw and target_index is not None: arg = list(arg) arg[target_index] = arg[target_index].obj() if not retval: - wrapped_fn(*arg, **kw) + fn(*arg, **kw) return interfaces.EXT_CONTINUE else: - return wrapped_fn(*arg, **kw) - fn = wrap + return fn(*arg, **kw) event_key = event_key.with_wrapper(wrap) if propagate: @@ -1560,17 +1557,14 @@ class AttributeEvents(event.Events): target.dispatch._active_history = True if not raw or not retval: - orig_fn = fn - def wrap(target, value, *arg): if not raw: target = target.obj() if not retval: - orig_fn(target, value, *arg) + fn(target, value, *arg) return value else: - return orig_fn(target, value, *arg) - fn = wrap + return fn(target, value, *arg) event_key = event_key.with_wrapper(wrap) event_key.base_listen(propagate=propagate) diff --git a/test/orm/test_events.py b/test/orm/test_events.py index a84ead0fae..e27294db4e 100644 --- a/test/orm/test_events.py +++ b/test/orm/test_events.py @@ -240,6 +240,7 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest): 'before_update', 'after_update', 'before_delete', 'after_delete']) + def test_before_after_only_collection(self): """before_update is called on parent for collection modifications, after_update is called even if no columns were updated. @@ -319,6 +320,41 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest): mapper(Address, addresses) eq_(canary, [User, Address]) +class DeclarativeEventListenTest(_RemoveListeners, fixtures.DeclarativeMappedTest): + run_setup_classes = "each" + run_deletes = None + + def test_inheritance_propagate_after_config(self): + # test [ticket:2949] + + class A(self.DeclarativeBasic): + __tablename__ = 'a' + id = Column(Integer, primary_key=True) + + class B(A): + pass + + listen = Mock() + event.listen(self.DeclarativeBasic, "load", listen, propagate=True) + + class C(B): + pass + + m1 = A.__mapper__.class_manager + m2 = B.__mapper__.class_manager + m3 = C.__mapper__.class_manager + a1 = A() + b1 = B() + c1 = C() + m3.dispatch.load(c1._sa_instance_state, "c") + m2.dispatch.load(b1._sa_instance_state, "b") + m1.dispatch.load(a1._sa_instance_state, "a") + eq_( + listen.mock_calls, + [call(c1, "c"), call(b1, "b"), call(a1, "a")] + ) + + class DeferredMapperEventsTest(_RemoveListeners, _fixtures.FixtureTest): """"test event listeners against unmapped classes. @@ -778,6 +814,27 @@ class RemovalTest(_fixtures.FixtureTest): m.dispatch.before_insert(m, None, attributes.instance_state(b1)) eq_(fn.call_count, 1) + def test_instance_event_listen_on_cls_before_map(self): + users = self.tables.users + + fn = Mock() + + class User(object): + pass + + event.listen(User, "load", fn) + m = mapper(User, users) + + u1 = User() + m.class_manager.dispatch.load(u1._sa_instance_state, "u1") + + event.remove(User, "load", fn) + + m.class_manager.dispatch.load(u1._sa_instance_state, "u2") + + eq_(fn.mock_calls, [call(u1, "u1")]) + + class RefreshTest(_fixtures.FixtureTest): run_inserts = None -- 2.47.3