diff options
| -rw-r--r-- | doc/build/changelog/changelog_09.rst | 14 | ||||
| -rw-r--r-- | lib/sqlalchemy/event/attr.py | 6 | ||||
| -rw-r--r-- | lib/sqlalchemy/event/registry.py | 26 | ||||
| -rw-r--r-- | test/base/test_events.py | 59 |
4 files changed, 90 insertions, 15 deletions
diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index 91e66bded..56be5b38e 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -14,6 +14,20 @@ :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 :tickets: 3151 diff --git a/lib/sqlalchemy/event/attr.py b/lib/sqlalchemy/event/attr.py index dba1063cf..be2a82208 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 ba2f671a3..217cf7d44 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 30b728cd3..913e1d3f5 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")]) |