- 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]

3 files changed, 80 insertions, 18 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 1db54d85c..62db3e699 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -15,6 +15,17 @@
     :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
 
         Fixed bug where the :class:`.AutomapBase` class of the
diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index a09154dd0..b2c356f24 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 a84ead0fa..e27294db4 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