]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
- Fixed bug that affected generally the same classes of event
authorMike Bayer <mike_mp@zzzcomputing.com>
Thu, 18 Sep 2014 21:49:07 +0000 (17:49 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Thu, 18 Sep 2014 21:49:07 +0000 (17:49 -0400)
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

doc/build/changelog/changelog_09.rst
lib/sqlalchemy/event/registry.py
lib/sqlalchemy/events.py
lib/sqlalchemy/orm/events.py
test/base/test_events.py
test/engine/test_execute.py
test/orm/test_attributes.py
test/orm/test_events.py

index c0d13e16db5b5a6015fcd1239a609b9e5a0a9c0e..7c75996a4bff47ee5ddac38d7646adb7633c72d7 100644 (file)
 .. 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
index 217cf7d44e517d9ece00876fd69994f479987016..5b422c401982d28e0e9b3a61dc746514235318da 100644 (file)
@@ -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)
index 1ecec51b68e0ae02e1e61a5a9bb6f9b99b1742f8..1ff35b8b06d11c77ba667a3ab8536f439df1473b 100644 (file)
@@ -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
 
index c50a7b062c95afbff0db12a03d014f17a0eef36d..9ea0dd834a73b814e58e944962688c119a2bbd7f 100644 (file)
@@ -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):
@@ -498,7 +500,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:
@@ -1493,7 +1496,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
index 913e1d3f5defd810ca3dbaf9942b9ef528b79bf2..89379961e7a96f7b45d89aab816020ab03cf75b5 100644 (file)
@@ -192,7 +192,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
@@ -205,48 +205,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,
@@ -1000,7 +1056,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)
@@ -1008,7 +1064,7 @@ class RemovalTest(fixtures.TestBase):
 
                 event_key.base_listen()
 
-            def event_one(self, value):
+            def event_one(self, x):
                 pass
 
         class Target(object):
@@ -1214,6 +1270,34 @@ class RemovalTest(fixtures.TestBase):
             t1.dispatch.event_one
         )
 
+    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()
 
index d8e1c655e9c8f59c8b67bc0cfd8c9541042ca42f..e14a4fd2a77aba6d708f37bfb39fddf25cb6e6f0 100644 (file)
@@ -1440,6 +1440,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):
index 46d5f86e504292b39fa7b698759b3344c644c16f..9c1f7a9856bcc4edbc2b9b11ddaae263726509b6 100644 (file)
@@ -2522,6 +2522,53 @@ 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=attributes.Event(
+                        Foo.data.impl, attributes.OP_REPLACE),
+                    target=f1, value=5),
+                call.append(
+                    initiator=attributes.Event(
+                        Foo.barlist.impl, attributes.OP_APPEND),
+                    target=f1,
+                    value=b1),
+                call.remove(
+                    initiator=attributes.Event(
+                        Foo.barlist.impl, attributes.OP_REMOVE),
+                    target=f1,
+                    value=b1)]
+        )
+
     def test_collection_link_events(self):
         class Foo(object):
             pass
@@ -2559,9 +2606,6 @@ class ListenerTest(fixtures.ORMTest):
         )
 
 
-
-
-
     def test_none_on_collection_event(self):
         """test that append/remove of None in collections emits events.
 
index e6efd6fb91f4cb440c616517c1061c62785a19c3..90429310260e4c93dd2e183bf01addc36634861b 100644 (file)
@@ -112,6 +112,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')
@@ -125,13 +126,15 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest):
         sess.flush()
         sess.delete(u)
         sess.flush()
-        eq_(canary,
-            ['init', 'before_insert',
-             'after_insert', 'expire',
-             'refresh',
-             'load',
-             'before_update', 'after_update', 'before_delete',
-             'after_delete'])
+        expected = [
+            'init', 'before_insert',
+            'after_insert', 'expire',
+            'refresh',
+            'load',
+            '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
@@ -1193,6 +1196,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()