- Fixed bug that affected many classes of event, particularly

ORM events but also engine events, where the usual logic of
"de duplicating" a redundant call to :func:`.event.listen`
with the same arguments would fail, for those events where the
listener function is wrapped.  An assertion would be hit within
registry.py.  This assertion has now been integrated into the
deduplication check, with the added bonus of a simpler means
of checking deduplication across the board.
fixes #3199

Conflicts:
	lib/sqlalchemy/event/registry.py
	test/base/test_events.py

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 91e66bded3b6eab17c190e262f71312197ad9286..56be5b38e7f3e572ab98d694b93468a9f6c93225 100644 (file)
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -13,6 +13,20 @@
 .. changelog::
     :version: 0.9.8
 
+    .. change::
+        :tags: bug, orm
+        :versions: 1.0.0
+        :tickets: 3199
+
+        Fixed bug that affected many classes of event, particularly
+        ORM events but also engine events, where the usual logic of
+        "de duplicating" a redundant call to :func:`.event.listen`
+        with the same arguments would fail, for those events where the
+        listener function is wrapped.  An assertion would be hit within
+        registry.py.  This assertion has now been integrated into the
+        deduplication check, with the added bonus of a simpler means
+        of checking deduplication across the board.
+
     .. change::
         :tags: bug, mssql
         :versions: 1.0.0

diff --git a/lib/sqlalchemy/event/attr.py b/lib/sqlalchemy/event/attr.py
index 7641b595a857e3083034b21d14b3222a0331cb22..7e786cc7ae5a2b501ff3ece6624530061d99c2c2 100644 (file)
--- a/lib/sqlalchemy/event/attr.py
+++ b/lib/sqlalchemy/event/attr.py
@@ -318,14 +318,12 @@ class _ListenerCollection(RefCollection, _CompoundListener):
         registry._stored_in_collection_multi(self, other, to_associate)
 
     def insert(self, event_key, propagate):
-        if event_key._listen_fn not in self.listeners:
-            event_key.prepend_to_list(self, self.listeners)
+        if event_key.prepend_to_list(self, self.listeners):
             if propagate:
                 self.propagate.add(event_key._listen_fn)
 
     def append(self, event_key, propagate):
-        if event_key._listen_fn not in self.listeners:
-            event_key.append_to_list(self, self.listeners)
+        if event_key.append_to_list(self, self.listeners):
             if propagate:
                 self.propagate.add(event_key._listen_fn)
 

diff --git a/lib/sqlalchemy/event/registry.py b/lib/sqlalchemy/event/registry.py
index a34de3cd7549bc0950402ef1dc6f7784a33b9ec1..28683d05ccd90f14faa2e14f6cb50e7ca6dd7051 100644 (file)
--- a/lib/sqlalchemy/event/registry.py
+++ b/lib/sqlalchemy/event/registry.py
@@ -71,13 +71,15 @@ def _stored_in_collection(event_key, owner):
     listen_ref = weakref.ref(event_key._listen_fn)
 
     if owner_ref in dispatch_reg:
-        assert dispatch_reg[owner_ref] == listen_ref
-    else:
-        dispatch_reg[owner_ref] = listen_ref
+        return False
+
+    dispatch_reg[owner_ref] = listen_ref
 
     listener_to_key = _collection_to_key[owner_ref]
     listener_to_key[listen_ref] = key
 
+    return True
+
 
 def _removed_from_collection(event_key, owner):
     key = event_key._key
@@ -229,18 +231,20 @@ class _EventKey(object):
     def _listen_fn(self):
         return self.fn_wrap or self.fn
 
-    def append_value_to_list(self, owner, list_, value):
-        _stored_in_collection(self, owner)
-        list_.append(value)
-
     def append_to_list(self, owner, list_):
-        _stored_in_collection(self, owner)
-        list_.append(self._listen_fn)
+        if _stored_in_collection(self, owner):
+            list_.append(self._listen_fn)
+            return True
+        else:
+            return False
 
     def remove_from_list(self, owner, list_):
         _removed_from_collection(self, owner)
         list_.remove(self._listen_fn)
 
     def prepend_to_list(self, owner, list_):
-        _stored_in_collection(self, owner)
-        list_.insert(0, self._listen_fn)
+        if _stored_in_collection(self, owner):
+            list_.insert(0, self._listen_fn)
+            return True
+        else:
+            return False

diff --git a/test/base/test_events.py b/test/base/test_events.py
index 4ae89fe1750ee4c54e5f4d1b3cd7c63353fb9f8f..3513f000d874d2a91dc0dc6d7b553999ff9e731b 100644 (file)
--- a/test/base/test_events.py
+++ b/test/base/test_events.py
@@ -985,6 +985,25 @@ class RemovalTest(fixtures.TestBase):
             dispatch = event.dispatcher(TargetEvents)
         return Target
 
+    def _wrapped_fixture(self):
+        class TargetEvents(event.Events):
+            @classmethod
+            def _listen(cls, event_key):
+                fn = event_key.fn
+
+                def adapt(value):
+                    fn("adapted " + value)
+                event_key = event_key.with_wrapper(adapt)
+
+                event_key.base_listen()
+
+            def event_one(self, value):
+                pass
+
+        class Target(object):
+            dispatch = event.dispatcher(TargetEvents)
+        return Target
+
     def test_clslevel(self):
         Target = self._fixture()
 
@@ -1145,3 +1164,43 @@ class RemovalTest(fixtures.TestBase):
         )
 
         event.remove(t1, "event_three", m1)
+
+    def test_double_event_nonwrapped(self):
+        Target = self._fixture()
+
+        listen_one = Mock()
+        t1 = Target()
+        event.listen(t1, "event_one", listen_one)
+        event.listen(t1, "event_one", listen_one)
+
+        t1.dispatch.event_one("t1")
+
+        # doubles are eliminated
+        eq_(listen_one.mock_calls, [call("t1")])
+
+        # only one remove needed
+        event.remove(t1, "event_one", listen_one)
+        t1.dispatch.event_one("t2")
+
+        eq_(listen_one.mock_calls, [call("t1")])
+
+    def test_double_event_wrapped(self):
+        # this is issue #3199
+        Target = self._wrapped_fixture()
+
+        listen_one = Mock()
+        t1 = Target()
+
+        event.listen(t1, "event_one", listen_one)
+        event.listen(t1, "event_one", listen_one)
+
+        t1.dispatch.event_one("t1")
+
+        # doubles are eliminated
+        eq_(listen_one.mock_calls, [call("adapted t1")])
+
+        # only one remove needed
+        event.remove(t1, "event_one", listen_one)
+        t1.dispatch.event_one("t2")
+
+        eq_(listen_one.mock_calls, [call("adapted t1")])