From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 12 Oct 2012 21:21:08 +0000 (-0400)
Subject:   - [feature] Improvements to event listening for
X-Git-Tag: rel_0_8_0b1~55
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c30e6d063e6dcec7d6f8ab28692049aaf387a5fa;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

  - [feature] Improvements to event listening for
    mapped classes allows that unmapped classes
    can be specified for instance- and mapper-events.
    The established events will be automatically
    set up on subclasses of that class when the
    propagate=True flag is passed, and the
    events will be set up for that class itself
    if and when it is ultimately mapped.
    [ticket:2585]

  - [bug] The instrumentation events class_instrument(),
    class_uninstrument(), and attribute_instrument()
    will now fire off only for descendant classes
    of the class assigned to listen().  Previously,
    an event listener would be assigned to listen
    for all classes in all cases regardless of the
    "target" argument passed.  [ticket:2590]
---

diff --git a/CHANGES b/CHANGES
index af4d1127e4..a376bbc3cc 100644
--- a/CHANGES
+++ b/CHANGES
@@ -81,6 +81,24 @@ underneath "0.7.xx".
     contains_eager()
     [ticket:2438] [ticket:1106]
 
+  - [feature] Improvements to event listening for
+    mapped classes allows that unmapped classes
+    can be specified for instance- and mapper-events.
+    The established events will be automatically
+    set up on subclasses of that class when the
+    propagate=True flag is passed, and the
+    events will be set up for that class itself
+    if and when it is ultimately mapped.
+    [ticket:2585]
+
+  - [bug] The instrumentation events class_instrument(),
+    class_uninstrument(), and attribute_instrument()
+    will now fire off only for descendant classes
+    of the class assigned to listen().  Previously,
+    an event listener would be assigned to listen
+    for all classes in all cases regardless of the
+    "target" argument passed.  [ticket:2590]
+
   - [bug] with_polymorphic() produces JOINs
     in the correct order and with correct inheriting
     tables in the case of sending multi-level
diff --git a/lib/sqlalchemy/event.py b/lib/sqlalchemy/event.py
index 633cb96f86..c702d9d34a 100644
--- a/lib/sqlalchemy/event.py
+++ b/lib/sqlalchemy/event.py
@@ -148,6 +148,12 @@ class _Dispatch(object):
             getattr(self, ls.name).\
                 for_modify(self)._update(ls, only_propagate=only_propagate)
 
+    @util.hybridmethod
+    def _clear(self):
+        for attr in dir(self):
+            if _is_event_name(attr):
+                getattr(self, attr).for_modify(self).clear()
+
 def _event_descriptors(target):
     return [getattr(target, k) for k in dir(target) if _is_event_name(k)]
 
@@ -170,7 +176,6 @@ def _create_dispatcher_class(cls, classname, bases, dict_):
     cls.dispatch = dispatch_cls = type("%sDispatch" % classname,
                                         (dispatch_base, ), {})
     dispatch_cls._listen = cls._listen
-    dispatch_cls._clear = cls._clear
 
     for k in dict_:
         if _is_event_name(k):
@@ -218,9 +223,7 @@ class Events(object):
 
     @classmethod
     def _clear(cls):
-        for attr in dir(cls.dispatch):
-            if _is_event_name(attr):
-                getattr(cls.dispatch, attr).clear()
+        cls.dispatch._clear()
 
 class _DispatchDescriptor(object):
     """Class-level attributes on :class:`._Dispatch` classes."""
diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index 9132ef8fa7..174652a15a 100644
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -10,6 +10,8 @@
 from .. import event, exc, util
 orm = util.importlater("sqlalchemy", "orm")
 import inspect
+import weakref
+
 
 class InstrumentationEvents(event.Events):
     """Events related to class instrumentation events.
@@ -17,9 +19,20 @@ class InstrumentationEvents(event.Events):
     The listeners here support being established against
     any new style class, that is any object that is a subclass
     of 'type'.  Events will then be fired off for events
-    against that class as well as all subclasses.
-    'type' itself is also accepted as a target
-    in which case the events fire for all classes.
+    against that class.  If the "propagate=True" flag is passed
+    to event.listen(), the event will fire off for subclasses
+    of that class as well.
+
+    The Python ``type`` builtin is also accepted as a target,
+    which when used has the effect of events being emitted
+    for all classes.
+
+    .. versionchanged:: 0.8 - events here will emit based
+       on comparing the incoming class to the type of class
+       passed to :func:`.event.listen`.  Previously, the
+       event would fire for any class unconditionally regardless
+       of what class was sent for listening, despite
+       documentation which stated the contrary.
 
     """
 
@@ -27,19 +40,38 @@ class InstrumentationEvents(event.Events):
     def _accept_with(cls, target):
         # TODO: there's no coverage for this
         if isinstance(target, type):
-            return orm.instrumentation._instrumentation_factory
+            return _InstrumentationEventsHold(target)
         else:
             return None
 
     @classmethod
     def _listen(cls, target, identifier, fn, propagate=False):
-        event.Events._listen(target, identifier, fn, propagate=propagate)
+
+        def listen(target_cls, *arg):
+            listen_cls = target()
+            if propagate and issubclass(target_cls, listen_cls):
+                return fn(target_cls, *arg)
+            elif not propagate and target_cls is listen_cls:
+                return fn(target_cls, *arg)
+
+        def remove(ref):
+            event.Events._remove(orm.instrumentation._instrumentation_factory,
+                                            identifier, listen)
+
+        target = weakref.ref(target.class_, remove)
+        event.Events._listen(orm.instrumentation._instrumentation_factory,
+                        identifier, listen)
 
     @classmethod
     def _remove(cls, identifier, target, fn):
         raise NotImplementedError("Removal of instrumentation events "
                                     "not yet implemented")
 
+    @classmethod
+    def _clear(cls):
+        super(InstrumentationEvents, cls)._clear()
+        orm.instrumentation._instrumentation_factory.dispatch._clear()
+
     def class_instrument(self, cls):
         """Called after the given class is instrumented.
 
@@ -60,6 +92,17 @@ class InstrumentationEvents(event.Events):
     def attribute_instrument(self, cls, key, inst):
         """Called when an attribute is instrumented."""
 
+class _InstrumentationEventsHold(object):
+    """temporary marker object used to transfer from _accept_with() to _listen()
+    on the InstrumentationEvents class.
+
+    """
+    def __init__(self, class_):
+        self.class_ = class_
+
+    dispatch = event.dispatcher(InstrumentationEvents)
+
+
 class InstanceEvents(event.Events):
     """Define events specific to object lifecycle.
 
@@ -94,8 +137,8 @@ class InstanceEvents(event.Events):
     available to the :func:`.event.listen` function.
 
     :param propagate=False: When True, the event listener should
-       be applied to all inheriting mappers as well as the
-       mapper which is the target of this listener.
+       be applied to all inheriting classes as well as the
+       class which is the target of this listener.
     :param raw=False: When True, the "target" argument passed
        to applicable event listener functions will be the
        instance's :class:`.InstanceState` management
@@ -117,6 +160,8 @@ class InstanceEvents(event.Events):
                 manager = orm.instrumentation.manager_of_class(target)
                 if manager:
                     return manager
+                else:
+                    return _InstanceEventsHold(target)
         return None
 
     @classmethod
@@ -136,6 +181,11 @@ class InstanceEvents(event.Events):
     def _remove(cls, identifier, target, fn):
         raise NotImplementedError("Removal of instance events not yet implemented")
 
+    @classmethod
+    def _clear(cls):
+        super(InstanceEvents, cls)._clear()
+        _InstanceEventsHold._clear()
+
     def first_init(self, manager, cls):
         """Called when the first instance of a particular mapping is called.
 
@@ -258,6 +308,51 @@ class InstanceEvents(event.Events):
 
         """
 
+class _EventsHold(object):
+    """Hold onto listeners against unmapped, uninstrumented classes.
+
+    Establish _listen() for that class' mapper/instrumentation when
+    those objects are created for that class.
+
+    """
+    def __init__(self, class_):
+        self.class_ = class_
+
+    @classmethod
+    def _clear(cls):
+        cls.all_holds.clear()
+
+    class HoldEvents(object):
+        @classmethod
+        def _listen(cls, target, identifier, fn, raw=False, propagate=False):
+            if target.class_ in target.all_holds:
+                collection = target.all_holds[target.class_]
+            else:
+                collection = target.all_holds[target.class_] = []
+
+            collection.append((target.class_, identifier, fn, raw, propagate))
+
+    @classmethod
+    def populate(cls, class_, subject):
+        for subclass in class_.__mro__:
+            if subclass in cls.all_holds:
+                if subclass is class_:
+                    collection = cls.all_holds.pop(subclass)
+                else:
+                    collection = cls.all_holds[subclass]
+                for target, ident, fn, raw, propagate in collection:
+                    if propagate or subclass is class_:
+                        subject.dispatch._listen(subject, ident, fn, raw, propagate)
+
+class _InstanceEventsHold(_EventsHold):
+    all_holds = weakref.WeakKeyDictionary()
+
+    class HoldInstanceEvents(_EventsHold.HoldEvents, InstanceEvents):
+        pass
+
+    dispatch = event.dispatcher(HoldInstanceEvents)
+
+
 class MapperEvents(event.Events):
     """Define events specific to mappings.
 
@@ -307,7 +402,8 @@ class MapperEvents(event.Events):
     available to the :func:`.event.listen` function.
 
     :param propagate=False: When True, the event listener should
-       be applied to all inheriting mappers as well as the
+       be applied to all inheriting mappers and/or the mappers of
+       inheriting classes, as well as any
        mapper which is the target of this listener.
     :param raw=False: When True, the "target" argument passed
        to applicable event listener functions will be the
@@ -337,7 +433,11 @@ class MapperEvents(event.Events):
             if issubclass(target, orm.Mapper):
                 return target
             else:
-                return orm.class_mapper(target, configure=False)
+                mapper = orm.util._mapper_or_none(target)
+                if mapper is not None:
+                    return mapper
+                else:
+                    return _MapperEventsHold(target)
         else:
             return target
 
@@ -371,6 +471,11 @@ class MapperEvents(event.Events):
         else:
             event.Events._listen(target, identifier, fn)
 
+    @classmethod
+    def _clear(cls):
+        super(MapperEvents, cls)._clear()
+        _MapperEventsHold._clear()
+
     def instrument_class(self, mapper, class_):
         """Receive a class when the mapper is first constructed,
         before instrumentation is applied to the mapped class.
@@ -906,6 +1011,14 @@ class MapperEvents(event.Events):
     def _remove(cls, identifier, target, fn):
         raise NotImplementedError("Removal of mapper events not yet implemented")
 
+class _MapperEventsHold(_EventsHold):
+    all_holds = weakref.WeakKeyDictionary()
+
+    class HoldMapperEvents(_EventsHold.HoldEvents, MapperEvents):
+        pass
+
+    dispatch = event.dispatcher(HoldMapperEvents)
+
 class SessionEvents(event.Events):
     """Define events specific to :class:`.Session` lifecycle.
 
diff --git a/lib/sqlalchemy/orm/instrumentation.py b/lib/sqlalchemy/orm/instrumentation.py
index 9a185c9ef0..0e828ce876 100644
--- a/lib/sqlalchemy/orm/instrumentation.py
+++ b/lib/sqlalchemy/orm/instrumentation.py
@@ -63,6 +63,12 @@ class ClassManager(dict):
         for base in self._bases:
             self.update(base)
 
+        events._InstanceEventsHold.populate(class_, self)
+
+        for basecls in class_.__mro__:
+            mgr = manager_of_class(basecls)
+            if mgr is not None:
+                self.dispatch._update(mgr.dispatch)
         self.manage()
         self._instrument_init()
 
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 4269e332fa..dfd8a12b7c 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -208,6 +208,7 @@ class Mapper(_InspectionAttr):
         # configure_mappers() until construction succeeds)
         _CONFIGURE_MUTEX.acquire()
         try:
+            events._MapperEventsHold.populate(class_, self)
             self._configure_inheritance()
             self._configure_legacy_instrument_class()
             self._configure_class_instrumentation()
@@ -659,10 +660,6 @@ class Mapper(_InspectionAttr):
             if ext not in super_extensions:
                 ext._adapt_listener(self, ext)
 
-        if self.inherits:
-            self.class_manager.dispatch._update(
-                        self.inherits.class_manager.dispatch)
-
     def _configure_class_instrumentation(self):
         """If this mapper is to be a primary mapper (i.e. the
         non_primary flag is not set), associate this Mapper with the
diff --git a/test/orm/test_events.py b/test/orm/test_events.py
index d8bef66d3c..6b903d9beb 100644
--- a/test/orm/test_events.py
+++ b/test/orm/test_events.py
@@ -8,8 +8,10 @@ from sqlalchemy.orm import mapper, relationship, \
     Mapper, column_property, \
     Session, sessionmaker, attributes
 from sqlalchemy.orm.instrumentation import ClassManager
+from sqlalchemy.orm import instrumentation, events
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
+from sqlalchemy.testing.util import gc_collect
 from test.orm import _fixtures
 from sqlalchemy import event
 
@@ -18,9 +20,10 @@ class _RemoveListeners(object):
     def teardown(self):
         # TODO: need to get remove() functionality
         # going
-        Mapper.dispatch._clear()
-        ClassManager.dispatch._clear()
-        Session.dispatch._clear()
+        events.MapperEvents._clear()
+        events.InstanceEvents._clear()
+        events.SessionEvents._clear()
+        events.InstrumentationEvents._clear()
         super(_RemoveListeners, self).teardown()
 
 
@@ -198,6 +201,47 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest):
             'before_update', 'after_update', 'before_delete',
             'after_delete'])
 
+    def test_inheritance_subclass_deferred(self):
+        users, addresses, User = (self.tables.users,
+                                self.tables.addresses,
+                                self.classes.User)
+
+
+        mapper(User, users)
+
+        canary1 = self.listen_all(User, propagate=True)
+        canary2 = self.listen_all(User)
+
+        class AdminUser(User):
+            pass
+        mapper(AdminUser, addresses, inherits=User)
+        canary3 = self.listen_all(AdminUser)
+
+        sess = create_session()
+        am = AdminUser(name='au1', email_address='au1@e1')
+        sess.add(am)
+        sess.flush()
+        am = sess.query(AdminUser).populate_existing().get(am.id)
+        sess.expunge_all()
+        am = sess.query(AdminUser).get(am.id)
+        am.name = 'au1 changed'
+        sess.flush()
+        sess.delete(am)
+        sess.flush()
+        eq_(canary1, ['init', 'before_insert', 'after_insert',
+            '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_(canary2, [])
+        eq_(canary3, ['init', 'before_insert', 'after_insert',
+            'translate_row', 'populate_instance','refresh',
+            'append_result', 'translate_row', 'create_instance'
+            , 'populate_instance', 'load', 'append_result',
+            'before_update', 'after_update', 'before_delete',
+            'after_delete'])
+
     def test_before_after_only_collection(self):
         """before_update is called on parent for collection modifications,
         after_update is called even if no columns were updated.
@@ -277,6 +321,219 @@ class MapperEventsTest(_RemoveListeners, _fixtures.FixtureTest):
         mapper(Address, addresses)
         eq_(canary, [User, Address])
 
+class DeferredMapperEventsTest(_RemoveListeners, _fixtures.FixtureTest):
+    """"test event listeners against unmapped classes.
+
+    This incurs special logic.  Note if we ever do the "remove" case,
+    it has to get all of these, too.
+
+    """
+    run_inserts = None
+
+    def test_deferred_map_event(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "before_insert", evt, raw=True)
+
+        m = mapper(User, users)
+        m.dispatch.before_insert(5)
+        eq_(canary, [5])
+
+    def test_deferred_map_event_subclass_propagate(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        class SubUser(User):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "before_insert", evt, propagate=True, raw=True)
+
+        m = mapper(SubUser, users)
+        m.dispatch.before_insert(5)
+        eq_(canary, [5])
+
+    def test_deferred_map_event_subclass_no_propagate(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        class SubUser(User):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "before_insert", evt, propagate=False)
+
+        m = mapper(SubUser, users)
+        m.dispatch.before_insert(5)
+        eq_(canary, [])
+
+    def test_deferred_instance_event_plain(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "load", evt, raw=True)
+
+        m = mapper(User, users)
+        m.class_manager.dispatch.load(5)
+        eq_(canary, [5])
+
+    def test_deferred_instance_event_subclass_propagate_subclass_only(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        class SubUser(User):
+            pass
+
+        class SubUser2(User):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "load", evt, propagate=True, raw=True)
+
+        m = mapper(SubUser, users)
+        m2 = mapper(SubUser2, users)
+
+        m.class_manager.dispatch.load(5)
+        eq_(canary, [5])
+
+        m2.class_manager.dispatch.load(5)
+        eq_(canary, [5, 5])
+
+    def test_deferred_instance_event_subclass_propagate_baseclass(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        class SubUser(User):
+            pass
+
+        class SubUser2(User):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "load", evt, propagate=True, raw=True)
+
+        m = mapper(SubUser, users)
+        m2 = mapper(User, users)
+
+        m.class_manager.dispatch.load(5)
+        eq_(canary, [5])
+
+        m2.class_manager.dispatch.load(5)
+        eq_(canary, [5, 5])
+
+        m3 = mapper(SubUser2, users)
+        m3.class_manager.dispatch.load(5)
+        eq_(canary, [5, 5, 5])
+
+    def test_deferred_instance_event_subclass_no_propagate(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        class SubUser(User):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "load", evt, propagate=False)
+
+        m = mapper(SubUser, users)
+        m.class_manager.dispatch.load(5)
+        eq_(canary, [])
+
+    def test_deferred_instrument_event(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "attribute_instrument", evt)
+
+        instrumentation._instrumentation_factory.dispatch.attribute_instrument(User)
+        eq_(canary, [User])
+
+    def test_isolation_instrument_event(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+        class Bar(object):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(Bar, "attribute_instrument", evt)
+
+        instrumentation._instrumentation_factory.dispatch.attribute_instrument(User)
+        eq_(canary, [])
+
+    @testing.requires.predictable_gc
+    def test_instrument_event_auto_remove(self):
+        class Bar(object):
+            pass
+
+        listeners = instrumentation._instrumentation_factory.dispatch.\
+                            attribute_instrument.listeners
+        assert not listeners
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(Bar, "attribute_instrument", evt)
+
+        eq_(len(listeners), 1)
+
+        del Bar
+        gc_collect()
+
+        assert not listeners
+
+
+    def test_deferred_instrument_event_subclass_propagate(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+        class SubUser(User):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "attribute_instrument", evt, propagate=True)
+
+        instrumentation._instrumentation_factory.dispatch.\
+                            attribute_instrument(SubUser)
+        eq_(canary, [SubUser])
+
+    def test_deferred_instrument_event_subclass_no_propagate(self):
+        users, User = (self.tables.users,
+                                self.classes.User)
+        class SubUser(User):
+            pass
+
+        canary = []
+        def evt(x):
+            canary.append(x)
+        event.listen(User, "attribute_instrument", evt, propagate=False)
+
+        mapper(SubUser, users)
+        instrumentation._instrumentation_factory.dispatch.attribute_instrument(5)
+        eq_(canary, [])
+
 
 class LoadTest(_fixtures.FixtureTest):
     run_inserts = None