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

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 1db54d85cc71d87d72aed0eba7b47488016ac1e3..62db3e6997fe9af22edd10d6e663ab20cc53691b 100644 (file)
--- 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 a09154dd003abe4b060a97909cbd4c86459a8c60..b2c356f24620c59e912dec47b68e6c1e6c725c8e 100644 (file)
--- 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 a84ead0fae040c95001dc9e5ab9c5bd049e9e5fc..e27294db4ec2e8ec8d2c7bf0551f007bf3a50bb1 100644 (file)
--- 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