- [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]

4 files changed, 136 insertions, 17 deletions

diff --git a/lib/sqlalchemy/event.py b/lib/sqlalchemy/event.py
index 633cb96f8..c702d9d34 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 9132ef8fa..174652a15 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 9a185c9ef..0e828ce87 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 4269e332f..dfd8a12b7 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