From: Mike Bayer Date: Thu, 18 Sep 2014 19:24:40 +0000 (-0400) Subject: - Fixed bug that affected many classes of event, particularly X-Git-Tag: rel_1_0_0b1~70^2~74 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9ae4db27b993fbd4666907cd11c2de3a41aee02f;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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 --- diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index 91e66bded3..56be5b38e7 100644 --- 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 dba1063cfd..be2a822083 100644 --- a/lib/sqlalchemy/event/attr.py +++ b/lib/sqlalchemy/event/attr.py @@ -319,14 +319,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 ba2f671a34..217cf7d44e 100644 --- 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_.appendleft(self._listen_fn) + if _stored_in_collection(self, owner): + list_.appendleft(self._listen_fn) + return True + else: + return False diff --git a/test/base/test_events.py b/test/base/test_events.py index 30b728cd39..913e1d3f5d 100644 --- a/test/base/test_events.py +++ b/test/base/test_events.py @@ -996,6 +996,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() @@ -1194,3 +1213,43 @@ class RemovalTest(fixtures.TestBase): "deque mutated during iteration", t1.dispatch.event_one ) + + 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")])