diff options
| -rw-r--r-- | doc/build/changelog/unreleased_12/4181.rst | 12 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/base.py | 16 | ||||
| -rw-r--r-- | lib/sqlalchemy/event/attr.py | 35 | ||||
| -rw-r--r-- | test/engine/test_execute.py | 153 |
4 files changed, 160 insertions, 56 deletions
diff --git a/doc/build/changelog/unreleased_12/4181.rst b/doc/build/changelog/unreleased_12/4181.rst new file mode 100644 index 000000000..877fe5ba6 --- /dev/null +++ b/doc/build/changelog/unreleased_12/4181.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, engine + :tickets: 4181 + + Fixed bug where events associated with an :class:`Engine` + at the class level would be doubled when the + :meth:`.Engine.execution_options` method were used. To + achieve this, the semi-private class :class:`.OptionEngine` + no longer accepts events directly at the class level + and will raise an error; the class only propagates class-level + events from its parent :class:`.Engine`. Instance-level + events continue to work as before. diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 4bae94317..aa9358cd6 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -2189,6 +2189,8 @@ class Engine(Connectable, log.Identified): class OptionEngine(Engine): + _sa_propagate_class_events = False + def __init__(self, proxied, execution_options): self._proxied = proxied self.url = proxied.url @@ -2196,7 +2198,21 @@ class OptionEngine(Engine): self.logging_name = proxied.logging_name self.echo = proxied.echo log.instance_logger(self, echoflag=self.echo) + + # note: this will propagate events that are assigned to the parent + # engine after this OptionEngine is created. Since we share + # the events of the parent we also disallow class-level events + # to apply to the OptionEngine class directly. + # + # the other way this can work would be to transfer existing + # events only, using: + # self.dispatch._update(proxied.dispatch) + # + # that might be more appropriate however it would be a behavioral + # change for logic that assigns events to the parent engine and + # would like it to take effect for the already-created sub-engine. self.dispatch = self.dispatch._join(proxied.dispatch) + self._execution_options = proxied._execution_options self.update_execution_options(**execution_options) diff --git a/lib/sqlalchemy/event/attr.py b/lib/sqlalchemy/event/attr.py index 1068257cb..efa8fab42 100644 --- a/lib/sqlalchemy/event/attr.py +++ b/lib/sqlalchemy/event/attr.py @@ -30,7 +30,7 @@ as well as support for subclass propagation (e.g. events assigned to """ from __future__ import absolute_import, with_statement - +from .. import exc from .. import util from ..util import threading from . import registry @@ -47,6 +47,20 @@ class RefCollection(util.MemoizedSlots): return weakref.ref(self, registry._collection_gced) +class _empty_collection(object): + def append(self, element): + pass + + def extend(self, other): + pass + + def __iter__(self): + return iter([]) + + def clear(self): + pass + + class _ClsLevelDispatch(RefCollection): """Class-level events on :class:`._Dispatch` classes.""" @@ -91,6 +105,9 @@ class _ClsLevelDispatch(RefCollection): target = event_key.dispatch_target assert isinstance(target, type), \ "Class-level Event targets must be classes." + if not getattr(target, '_sa_propagate_class_events', True): + raise exc.InvalidRequestError( + "Can't assign an event directly to the %s class" % target) stack = [target] while stack: cls = stack.pop(0) @@ -99,7 +116,7 @@ class _ClsLevelDispatch(RefCollection): self.update_subclass(cls) else: if cls not in self._clslevel: - self._clslevel[cls] = collections.deque() + self._assign_cls_collection(cls) self._clslevel[cls].appendleft(event_key._listen_fn) registry._stored_in_collection(event_key, self) @@ -107,7 +124,9 @@ class _ClsLevelDispatch(RefCollection): target = event_key.dispatch_target assert isinstance(target, type), \ "Class-level Event targets must be classes." - + if not getattr(target, '_sa_propagate_class_events', True): + raise exc.InvalidRequestError( + "Can't assign an event directly to the %s class" % target) stack = [target] while stack: cls = stack.pop(0) @@ -116,13 +135,19 @@ class _ClsLevelDispatch(RefCollection): self.update_subclass(cls) else: if cls not in self._clslevel: - self._clslevel[cls] = collections.deque() + self._assign_cls_collection(cls) self._clslevel[cls].append(event_key._listen_fn) registry._stored_in_collection(event_key, self) + def _assign_cls_collection(self, target): + if getattr(target, '_sa_propagate_class_events', True): + self._clslevel[target] = collections.deque() + else: + self._clslevel[target] = _empty_collection() + def update_subclass(self, target): if target not in self._clslevel: - self._clslevel[target] = collections.deque() + self._assign_cls_collection(target) clslevel = self._clslevel[target] for cls in target.__mro__[1:]: if cls in self._clslevel: diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index 1b7934794..e0727f770 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -511,57 +511,6 @@ class ExecuteTest(fixtures.TestBase): is_(eng.pool, eng2.pool) @testing.requires.ad_hoc_engines - def test_generative_engine_event_dispatch(self): - canary = [] - - def l1(*arg, **kw): - canary.append("l1") - - def l2(*arg, **kw): - canary.append("l2") - - def l3(*arg, **kw): - canary.append("l3") - - eng = engines.testing_engine(options={'execution_options': - {'base': 'x1'}}) - event.listen(eng, "before_execute", l1) - - eng1 = eng.execution_options(foo="b1") - event.listen(eng, "before_execute", l2) - event.listen(eng1, "before_execute", l3) - - eng.execute(select([1])).close() - eng1.execute(select([1])).close() - - eq_(canary, ["l1", "l2", "l3", "l1", "l2"]) - - @testing.requires.ad_hoc_engines - def test_dispose_event(self): - canary = Mock() - eng = create_engine(testing.db.url) - event.listen(eng, "engine_disposed", canary) - - conn = eng.connect() - conn.close() - eng.dispose() - - conn = eng.connect() - conn.close() - - eq_( - canary.mock_calls, - [call(eng)] - ) - - eng.dispose() - - eq_( - canary.mock_calls, - [call(eng), call(eng)] - ) - - @testing.requires.ad_hoc_engines def test_autocommit_option_no_issue_first_connect(self): eng = create_engine(testing.db.url) eng.update_execution_options(autocommit=True) @@ -1387,6 +1336,108 @@ class EngineEventsTest(fixtures.TestBase): eq_(c3._execution_options, {'foo': 'bar', 'bar': 'bat'}) eq_(canary, ['execute', 'cursor_execute']) + @testing.requires.ad_hoc_engines + def test_generative_engine_event_dispatch(self): + canary = [] + + def l1(*arg, **kw): + canary.append("l1") + + def l2(*arg, **kw): + canary.append("l2") + + def l3(*arg, **kw): + canary.append("l3") + + eng = engines.testing_engine(options={'execution_options': + {'base': 'x1'}}) + event.listen(eng, "before_execute", l1) + + eng1 = eng.execution_options(foo="b1") + event.listen(eng, "before_execute", l2) + event.listen(eng1, "before_execute", l3) + + eng.execute(select([1])).close() + + eq_(canary, ["l1", "l2"]) + + eng1.execute(select([1])).close() + + eq_(canary, ["l1", "l2", "l3", "l1", "l2"]) + + @testing.requires.ad_hoc_engines + def test_clslevel_engine_event_options(self): + canary = [] + + def l1(*arg, **kw): + canary.append("l1") + + def l2(*arg, **kw): + canary.append("l2") + + def l3(*arg, **kw): + canary.append("l3") + + def l4(*arg, **kw): + canary.append("l4") + + event.listen(Engine, "before_execute", l1) + + eng = engines.testing_engine(options={'execution_options': + {'base': 'x1'}}) + event.listen(eng, "before_execute", l2) + + eng1 = eng.execution_options(foo="b1") + event.listen(eng, "before_execute", l3) + event.listen(eng1, "before_execute", l4) + + eng.execute(select([1])).close() + + eq_(canary, ["l1", "l2", "l3"]) + + eng1.execute(select([1])).close() + + eq_(canary, ["l1", "l2", "l3", "l4", "l1", "l2", "l3"]) + + @testing.requires.ad_hoc_engines + def test_cant_listen_to_option_engine(self): + from sqlalchemy.engine import base + + def evt(*arg, **kw): + pass + + assert_raises_message( + tsa.exc.InvalidRequestError, + r"Can't assign an event directly to the " + "<class 'sqlalchemy.engine.base.OptionEngine'> class", + event.listen, base.OptionEngine, "before_cursor_execute", evt + ) + + @testing.requires.ad_hoc_engines + def test_dispose_event(self): + canary = Mock() + eng = create_engine(testing.db.url) + event.listen(eng, "engine_disposed", canary) + + conn = eng.connect() + conn.close() + eng.dispose() + + conn = eng.connect() + conn.close() + + eq_( + canary.mock_calls, + [call(eng)] + ) + + eng.dispose() + + eq_( + canary.mock_calls, + [call(eng), call(eng)] + ) + def test_retval_flag(self): canary = [] |