From: Mike Bayer Date: Thu, 18 Sep 2014 21:49:07 +0000 (-0400) Subject: - Fixed bug that affected generally the same classes of event X-Git-Tag: rel_0_9_8~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=912ebaa8b02f05d6148e5a3c5194fa9d2b3dc36a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Fixed bug that affected generally the same classes of event as that of :ticket:`3199`, when the ``named=True`` parameter would be used. Some events would fail to register, and others would not invoke the event arguments correctly, generally in the case of when an event was "wrapped" for adaption in some other way. The "named" mechanics have been rearranged to not interfere with the argument signature expected by internal wrapper functions. fixes #3197 Conflicts: test/base/test_events.py test/orm/test_attributes.py test/orm/test_events.py --- diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index c0d13e16db..7c75996a4b 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -13,6 +13,19 @@ .. changelog:: :version: 0.9.8 + .. change:: + :tags: bug, orm, engine + :versions: 1.0.0 + :tickets: 3197 + + Fixed bug that affected generally the same classes of event + as that of :ticket:`3199`, when the ``named=True`` parameter + would be used. Some events would fail to register, and others + would not invoke the event arguments correctly, generally in the + case of when an event was "wrapped" for adaption in some other way. + The "named" mechanics have been rearranged to not interfere with + the argument signature expected by internal wrapper functions. + .. change:: :tags: bug, declarative :versions: 1.0.0 diff --git a/lib/sqlalchemy/event/registry.py b/lib/sqlalchemy/event/registry.py index 28683d05cc..146ad3d75b 100644 --- a/lib/sqlalchemy/event/registry.py +++ b/lib/sqlalchemy/event/registry.py @@ -182,6 +182,17 @@ class _EventKey(object): def listen(self, *args, **kw): once = kw.pop("once", False) + named = kw.pop("named", False) + + target, identifier, fn = \ + self.dispatch_target, self.identifier, self._listen_fn + + dispatch_descriptor = getattr(target.dispatch, identifier) + + adjusted_fn = dispatch_descriptor._adjust_fn_spec(fn, named) + + self = self.with_wrapper(adjusted_fn) + if once: self.with_wrapper( util.only_once(self._listen_fn)).listen(*args, **kw) @@ -217,9 +228,6 @@ class _EventKey(object): dispatch_descriptor = getattr(target.dispatch, identifier) - fn = dispatch_descriptor._adjust_fn_spec(fn, named) - self = self.with_wrapper(fn) - if insert: dispatch_descriptor.\ for_modify(target.dispatch).insert(self, propagate) diff --git a/lib/sqlalchemy/events.py b/lib/sqlalchemy/events.py index 42bbbfc0fb..39d707d75c 100644 --- a/lib/sqlalchemy/events.py +++ b/lib/sqlalchemy/events.py @@ -470,7 +470,8 @@ class ConnectionEvents(event.Events): @classmethod def _listen(cls, event_key, retval=False): target, identifier, fn = \ - event_key.dispatch_target, event_key.identifier, event_key.fn + event_key.dispatch_target, event_key.identifier, \ + event_key._listen_fn target._has_events = True diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py index d94bd715ef..cf6b49c23f 100644 --- a/lib/sqlalchemy/orm/events.py +++ b/lib/sqlalchemy/orm/events.py @@ -61,7 +61,8 @@ class InstrumentationEvents(event.Events): @classmethod def _listen(cls, event_key, propagate=True, **kw): target, identifier, fn = \ - event_key.dispatch_target, event_key.identifier, event_key.fn + event_key.dispatch_target, event_key.identifier, \ + event_key._listen_fn def listen(target_cls, *arg): listen_cls = target() @@ -192,7 +193,8 @@ class InstanceEvents(event.Events): @classmethod def _listen(cls, event_key, raw=False, propagate=False, **kw): target, identifier, fn = \ - event_key.dispatch_target, event_key.identifier, event_key.fn + event_key.dispatch_target, event_key.identifier, \ + event_key._listen_fn if not raw: def wrap(state, *arg, **kw): @@ -515,7 +517,8 @@ class MapperEvents(event.Events): def _listen( cls, event_key, raw=False, retval=False, propagate=False, **kw): target, identifier, fn = \ - event_key.dispatch_target, event_key.identifier, event_key.fn + event_key.dispatch_target, event_key.identifier, \ + event_key._listen_fn if identifier in ("before_configured", "after_configured") and \ target is not mapperlib.Mapper: @@ -1649,7 +1652,8 @@ class AttributeEvents(event.Events): propagate=False): target, identifier, fn = \ - event_key.dispatch_target, event_key.identifier, event_key.fn + event_key.dispatch_target, event_key.identifier, \ + event_key._listen_fn if active_history: target.dispatch._active_history = True diff --git a/test/base/test_events.py b/test/base/test_events.py index 3513f000d8..2489ebb3e6 100644 --- a/test/base/test_events.py +++ b/test/base/test_events.py @@ -190,7 +190,7 @@ class EventsTest(fixtures.TestBase): class NamedCallTest(fixtures.TestBase): - def setUp(self): + def _fixture(self): class TargetEventsOne(event.Events): def event_one(self, x, y): pass @@ -203,48 +203,104 @@ class NamedCallTest(fixtures.TestBase): class TargetOne(object): dispatch = event.dispatcher(TargetEventsOne) - self.TargetOne = TargetOne + return TargetOne - def tearDown(self): - event.base._remove_dispatcher(self.TargetOne.__dict__['dispatch'].events) + def _wrapped_fixture(self): + class TargetEvents(event.Events): + @classmethod + def _listen(cls, event_key): + fn = event_key._listen_fn + + def adapt(*args): + fn(*["adapted %s" % arg for arg in args]) + event_key = event_key.with_wrapper(adapt) + + event_key.base_listen() + + def event_one(self, x, y): + pass + + def event_five(self, x, y, z, q): + pass + class Target(object): + dispatch = event.dispatcher(TargetEvents) + return Target def test_kw_accept(self): + TargetOne = self._fixture() + canary = Mock() - @event.listens_for(self.TargetOne, "event_one", named=True) + @event.listens_for(TargetOne, "event_one", named=True) def handler1(**kw): canary(kw) - self.TargetOne().dispatch.event_one(4, 5) + TargetOne().dispatch.event_one(4, 5) eq_( canary.mock_calls, [call({"x": 4, "y": 5})] ) + def test_kw_accept_wrapped(self): + TargetOne = self._wrapped_fixture() + + canary = Mock() + + @event.listens_for(TargetOne, "event_one", named=True) + def handler1(**kw): + canary(kw) + + TargetOne().dispatch.event_one(4, 5) + + eq_( + canary.mock_calls, + [call({'y': 'adapted 5', 'x': 'adapted 4'})] + ) + def test_partial_kw_accept(self): + TargetOne = self._fixture() + canary = Mock() - @event.listens_for(self.TargetOne, "event_five", named=True) + @event.listens_for(TargetOne, "event_five", named=True) def handler1(z, y, **kw): canary(z, y, kw) - self.TargetOne().dispatch.event_five(4, 5, 6, 7) + TargetOne().dispatch.event_five(4, 5, 6, 7) eq_( canary.mock_calls, [call(6, 5, {"x": 4, "q": 7})] ) + def test_partial_kw_accept_wrapped(self): + TargetOne = self._wrapped_fixture() + + canary = Mock() + + @event.listens_for(TargetOne, "event_five", named=True) + def handler1(z, y, **kw): + canary(z, y, kw) + + TargetOne().dispatch.event_five(4, 5, 6, 7) + + eq_( + canary.mock_calls, + [call('adapted 6', 'adapted 5', + {'q': 'adapted 7', 'x': 'adapted 4'})] + ) + def test_kw_accept_plus_kw(self): + TargetOne = self._fixture() canary = Mock() - @event.listens_for(self.TargetOne, "event_two", named=True) + @event.listens_for(TargetOne, "event_two", named=True) def handler1(**kw): canary(kw) - self.TargetOne().dispatch.event_two(4, 5, z=8, q=5) + TargetOne().dispatch.event_two(4, 5, z=8, q=5) eq_( canary.mock_calls, @@ -989,7 +1045,7 @@ class RemovalTest(fixtures.TestBase): class TargetEvents(event.Events): @classmethod def _listen(cls, event_key): - fn = event_key.fn + fn = event_key._listen_fn def adapt(value): fn("adapted " + value) @@ -997,7 +1053,7 @@ class RemovalTest(fixtures.TestBase): event_key.base_listen() - def event_one(self, value): + def event_one(self, x): pass class Target(object): @@ -1165,6 +1221,34 @@ class RemovalTest(fixtures.TestBase): event.remove(t1, "event_three", m1) + def test_remove_plain_named(self): + Target = self._fixture() + + listen_one = Mock() + t1 = Target() + event.listen(t1, "event_one", listen_one, named=True) + t1.dispatch.event_one("t1") + + eq_(listen_one.mock_calls, [call(x="t1")]) + event.remove(t1, "event_one", listen_one) + t1.dispatch.event_one("t2") + + eq_(listen_one.mock_calls, [call(x="t1")]) + + def test_remove_wrapped_named(self): + Target = self._wrapped_fixture() + + listen_one = Mock() + t1 = Target() + event.listen(t1, "event_one", listen_one, named=True) + t1.dispatch.event_one("t1") + + eq_(listen_one.mock_calls, [call(x="adapted t1")]) + event.remove(t1, "event_one", listen_one) + t1.dispatch.event_one("t2") + + eq_(listen_one.mock_calls, [call(x="adapted t1")]) + def test_double_event_nonwrapped(self): Target = self._fixture() diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index 42fbe20fec..dbdb68e615 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -1439,6 +1439,48 @@ class EngineEventsTest(fixtures.TestBase): 'begin', 'execute', 'cursor_execute', 'commit', ]) + def test_transactional_named(self): + canary = [] + + def tracker(name): + def go(*args, **kw): + canary.append((name, set(kw))) + return go + + engine = engines.testing_engine() + event.listen(engine, 'before_execute', tracker('execute'), named=True) + event.listen( + engine, 'before_cursor_execute', + tracker('cursor_execute'), named=True) + event.listen(engine, 'begin', tracker('begin'), named=True) + event.listen(engine, 'commit', tracker('commit'), named=True) + event.listen(engine, 'rollback', tracker('rollback'), named=True) + + conn = engine.connect() + trans = conn.begin() + conn.execute(select([1])) + trans.rollback() + trans = conn.begin() + conn.execute(select([1])) + trans.commit() + + eq_( + canary, [ + ('begin', set(['conn', ])), + ('execute', set([ + 'conn', 'clauseelement', 'multiparams', 'params'])), + ('cursor_execute', set([ + 'conn', 'cursor', 'executemany', + 'statement', 'parameters', 'context'])), + ('rollback', set(['conn', ])), ('begin', set(['conn', ])), + ('execute', set([ + 'conn', 'clauseelement', 'multiparams', 'params'])), + ('cursor_execute', set([ + 'conn', 'cursor', 'executemany', 'statement', + 'parameters', 'context'])), + ('commit', set(['conn', ]))] + ) + @testing.requires.savepoints @testing.requires.two_phase_transactions def test_transactional_advanced(self): diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index b44f883c93..87ef11d13a 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -2486,6 +2486,50 @@ class ListenerTest(fixtures.ORMTest): f1.barset.add(b1) assert f1.barset.pop().data == 'some bar appended' + def test_named(self): + canary = Mock() + + class Foo(object): + pass + + class Bar(object): + pass + + instrumentation.register_class(Foo) + instrumentation.register_class(Bar) + attributes.register_attribute( + Foo, 'data', uselist=False, + useobject=False) + attributes.register_attribute( + Foo, 'barlist', uselist=True, + useobject=True) + + event.listen(Foo.data, 'set', canary.set, named=True) + event.listen(Foo.barlist, 'append', canary.append, named=True) + event.listen(Foo.barlist, 'remove', canary.remove, named=True) + + f1 = Foo() + b1 = Bar() + f1.data = 5 + f1.barlist.append(b1) + f1.barlist.remove(b1) + eq_( + canary.mock_calls, + [ + call.set( + oldvalue=attributes.NO_VALUE, + initiator=canary.mock_calls[0][2]['initiator'], + target=f1, value=5), + call.append( + initiator=canary.mock_calls[1][2]['initiator'], + target=f1, + value=b1), + call.remove( + initiator=canary.mock_calls[2][2]['initiator'], + target=f1, + value=b1)] + ) + def test_none_on_collection_event(self): """test that append/remove of None in collections emits events. diff --git a/test/orm/test_events.py b/test/orm/test_events.py index 8410dee51f..39d5c8eafa 100644 --- a/test/orm/test_events.py +++ b/test/orm/test_events.py @@ -116,6 +116,7 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest): mapper(User, users) canary = self.listen_all(User) + named_canary = self.listen_all(User, named=True) sess = create_session() u = User(name='u1') @@ -129,14 +130,18 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest): sess.flush() sess.delete(u) sess.flush() - eq_(canary, - ['init', 'before_insert', - 'after_insert', 'expire', 'translate_row', - 'populate_instance', 'refresh', - 'append_result', 'translate_row', 'create_instance', - 'populate_instance', 'load', 'append_result', - 'before_update', 'after_update', 'before_delete', - 'after_delete']) + + expected = [ + 'init', 'before_insert', + 'after_insert', 'expire', 'translate_row', + 'populate_instance', 'refresh', + 'append_result', 'translate_row', 'create_instance', + 'populate_instance', 'load', 'append_result', + 'before_update', 'after_update', 'before_delete', + 'after_delete'] + + eq_(canary, expected) + eq_(named_canary, expected) def test_insert_before_configured(self): users, User = self.tables.users, self.classes.User @@ -1221,6 +1226,7 @@ class SessionEventsTest(_RemoveListeners, _fixtures.FixtureTest): 'before_commit', 'after_commit','after_transaction_end'] ) + def test_rollback_hook(self): User, users = self.classes.User, self.tables.users sess, canary = self._listener_fixture()