Fixed bug in ORM-level event registration where the "raw" or

"propagate" flags could potentially be mis-configured in some
"unmapped base class" configurations.  Also in 0.8.3.
[ticket:2786]

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index f266b2e6effffe37e3cf9934e6d0a3a8a225ef70..a626bb31aa61cde2ded2cec3e68f27ab3c69b8ce 100644 (file)
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -6,6 +6,14 @@
 .. changelog::
     :version: 0.8.3
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 2786
+
+        Fixed bug in ORM-level event registration where the "raw" or
+        "propagate" flags could potentially be mis-configured in some
+        "unmapped base class" configurations.
+
     .. change::
         :tags: bug, sql
         :tickets: 2784

diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index cea07bcf0342804c9835e65fd527bdb5a98202e8..2f82c1c965ae2b62c80c6e03b45fd10e81cbd1f4 100644 (file)
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -355,14 +355,17 @@ class _EventsHold(object):
     def populate(cls, class_, subject):
         for subclass in class_.__mro__:
             if subclass in cls.all_holds:
-                if subclass is class_:
-                    collection = cls.all_holds.pop(subclass)
-                else:
-                    collection = cls.all_holds[subclass]
+                collection = cls.all_holds[subclass]
                 for ident, fn, raw, propagate in collection:
                     if propagate or subclass is class_:
+                        # since we can't be sure in what order different classes
+                        # in a hierarchy are triggered with populate(),
+                        # we rely upon _EventsHold for all event
+                        # assignment, instead of using the generic propagate
+                        # flag.
                         subject.dispatch._listen(subject, ident,
-                                                        fn, raw, propagate)
+                                                        fn, raw=raw,
+                                                        propagate=False)
 
 
 class _InstanceEventsHold(_EventsHold):

diff --git a/test/orm/test_events.py b/test/orm/test_events.py
index 2f91f5c8324a0c7791d946467f8e27c097a7a612..e8c9f34ee76927135d2d5099d4fd051630ae43fa 100644 (file)
--- a/test/orm/test_events.py
+++ b/test/orm/test_events.py
@@ -14,7 +14,7 @@ from sqlalchemy.testing import fixtures
 from sqlalchemy.testing.util import gc_collect
 from test.orm import _fixtures
 from sqlalchemy import event
-
+from sqlalchemy.testing.mock import Mock, call
 
 class _RemoveListeners(object):
     def teardown(self):
@@ -362,14 +362,25 @@ class DeferredMapperEventsTest(_RemoveListeners, _fixtures.FixtureTest):
         class SubUser(User):
             pass
 
-        canary = []
-        def evt(x):
+        class SubSubUser(SubUser):
+            pass
+
+        canary = Mock()
+        def evt(x, y, z):
             canary.append(x)
-        event.listen(User, "before_insert", evt, propagate=True, raw=True)
+        event.listen(User, "before_insert", canary, propagate=True, raw=True)
 
         m = mapper(SubUser, users)
-        m.dispatch.before_insert(5)
-        eq_(canary, [5])
+        m.dispatch.before_insert(5, 6, 7)
+        eq_(canary.mock_calls,
+                [call(5, 6, 7)])
+
+        m2 = mapper(SubSubUser, users)
+
+        m2.dispatch.before_insert(8, 9, 10)
+        eq_(canary.mock_calls,
+                [call(5, 6, 7), call(8, 9, 10)])
+
 
     def test_deferred_map_event_subclass_no_propagate(self):
         """
@@ -416,6 +427,35 @@ class DeferredMapperEventsTest(_RemoveListeners, _fixtures.FixtureTest):
         m.dispatch.before_insert(5)
         eq_(canary, [5])
 
+    def test_deferred_map_event_subclass_post_mapping_propagate_two(self):
+        """
+        1. map only subclass of class
+        2. mapper event listen on class, w propagate
+        3. event fire should receive event
+
+        """
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        class SubUser(User):
+            pass
+
+        class SubSubUser(SubUser):
+            pass
+
+        m = mapper(SubUser, users)
+
+        canary = Mock()
+        event.listen(User, "before_insert", canary, propagate=True, raw=True)
+
+        m2 = mapper(SubSubUser, users)
+
+        m.dispatch.before_insert(5, 6, 7)
+        eq_(canary.mock_calls, [call(5, 6, 7)])
+
+        m2.dispatch.before_insert(8, 9, 10)
+        eq_(canary.mock_calls, [call(5, 6, 7), call(8, 9, 10)])
+
     def test_deferred_instance_event_subclass_post_mapping_propagate(self):
         """
         1. map only subclass of class
@@ -507,23 +547,25 @@ class DeferredMapperEventsTest(_RemoveListeners, _fixtures.FixtureTest):
         class SubUser2(User):
             pass
 
-        canary = []
-        def evt(x):
-            canary.append(x)
-        event.listen(User, "load", evt, propagate=True, raw=True)
+        canary = Mock()
+        event.listen(User, "load", canary, propagate=True, raw=False)
 
+        # reversing these fixes....
         m = mapper(SubUser, users)
         m2 = mapper(User, users)
 
-        m.class_manager.dispatch.load(5)
-        eq_(canary, [5])
+        instance = Mock()
+        m.class_manager.dispatch.load(instance)
 
-        m2.class_manager.dispatch.load(5)
-        eq_(canary, [5, 5])
+        eq_(canary.mock_calls, [call(instance.obj())])
+
+        m2.class_manager.dispatch.load(instance)
+        eq_(canary.mock_calls, [call(instance.obj()), call(instance.obj())])
 
         m3 = mapper(SubUser2, users)
-        m3.class_manager.dispatch.load(5)
-        eq_(canary, [5, 5, 5])
+        m3.class_manager.dispatch.load(instance)
+        eq_(canary.mock_calls, [call(instance.obj()),
+                        call(instance.obj()), call(instance.obj())])
 
     def test_deferred_instance_event_subclass_no_propagate(self):
         """