From 90d992f2da1edb8ddfff3fd33d69c6ba7307d2a0 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 18 Jul 2013 23:17:33 -0400
Subject: [PATCH] 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]

---
 doc/build/changelog/changelog_08.rst |  8 +++
 lib/sqlalchemy/orm/events.py         | 13 +++--
 test/orm/test_events.py              | 74 ++++++++++++++++++++++------
 3 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index f266b2e6ef..a626bb31aa 100644
--- 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 cea07bcf03..2f82c1c965 100644
--- 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 2f91f5c832..e8c9f34ee7 100644
--- 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):
         """
-- 
2.47.2