- Renamed on_reconstitute to @reconstructor and reconstruct_instance

- Moved @reconstructor hooking to mapper
- Expanded reconstructor tests, docs

4 files changed, 131 insertions, 124 deletions

diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py
index 425a41b37..e405d76a2 100644
--- a/lib/sqlalchemy/orm/__init__.py
+++ b/lib/sqlalchemy/orm/__init__.py
@@ -44,6 +44,7 @@ from sqlalchemy.orm.properties import (
      SynonymProperty,
      )
 from sqlalchemy.orm import mapper as mapperlib
+from sqlalchemy.orm.mapper import reconstructor
 from sqlalchemy.orm import strategies
 from sqlalchemy.orm.query import AliasOption, Query
 from sqlalchemy.sql import util as sql_util
@@ -83,6 +84,7 @@ __all__ = (
     'object_mapper',
     'object_session',
     'polymorphic_union',
+    'reconstructor',
     'relation',
     'scoped_session',
     'sessionmaker',
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 40878be76..9ebe9f79f 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -3,14 +3,10 @@
 #
 # This module is part of SQLAlchemy and is released under
 # the MIT License: http://www.opensource.org/licenses/mit-license.php
-"""
-
-Defines SQLAlchemy's system of class instrumentation.
+"""Defines SQLAlchemy's system of class instrumentation..
 
-This module is usually not visible to user applications, but forms
-a large part of the ORM's interactivity.  The primary "public"
-function is the ``on_reconstitute`` decorator which is described in
-the main mapper documentation.
+This module is usually not directly visible to user applications, but
+defines a large part of the ORM's interactivity.
 
 SQLA's instrumentation system is completely customizable, in which
 case an understanding of the general mechanics of this module is helpful.
@@ -26,7 +22,6 @@ from sqlalchemy import util
 from sqlalchemy.util import EMPTY_SET
 from sqlalchemy.orm import interfaces, collections, exc
 import sqlalchemy.exceptions as sa_exc
-import types
 
 # lazy imports
 _entity_info = None
@@ -1056,10 +1051,6 @@ class ClassManager(dict):
         self._instantiable = False
         self.events = self.event_registry_factory()
 
-        for key, meth in util.iterate_attributes(class_):
-            if isinstance(meth, types.FunctionType) and hasattr(meth, '__sa_reconstitute__'):
-                self.events.add_listener('on_load', meth)
-
     def instantiable(self, boolean):
         # experiment, probably won't stay in this form
         assert boolean ^ self._instantiable, (boolean, self._instantiable)
@@ -1465,19 +1456,6 @@ def del_attribute(instance, key):
 def is_instrumented(instance, key):
     return manager_of_class(instance.__class__).is_instrumented(key, search=True)
 
-def on_reconstitute(fn):
-    """Decorate a method as the 'reconstitute' hook.
-    
-    This method will be called based on the 'on_load' event hook.
-    
-    Note that when using ORM mappers, this method is equivalent
-    to MapperExtension.on_reconstitute().
-
-    """
-    fn.__sa_reconstitute__ = True
-    return fn
-    
-    
 class InstrumentationRegistry(object):
     """Private instrumentation registration singleton."""
 
diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 283bc10a5..0b60483a3 100644
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -76,10 +76,10 @@ class MapperExtension(object):
         """Perform pre-processing on the given result row and return a
         new row instance.
 
-        This is called when the mapper first receives a row, before 
+        This is called when the mapper first receives a row, before
         the object identity or the instance itself has been derived
         from that row.
-        
+
         """
         return EXT_CONTINUE
 
@@ -143,41 +143,40 @@ class MapperExtension(object):
     def populate_instance(self, mapper, selectcontext, row, instance, **flags):
         """Receive an instance before that instance has
         its attributes populated.
-        
+
         This usually corresponds to a newly loaded instance but may
         also correspond to an already-loaded instance which has
-        unloaded attributes to be populated.  The method may be 
-        called many times for a single instance, as multiple
-        result rows are used to populate eagerly loaded collections.
-
-        If this method returns EXT_CONTINUE, instance
-        population will proceed normally.  If any other value or None
-        is returned, instance population will not proceed, giving this
-        extension an opportunity to populate the instance itself, if
-        desired.
-        
-        As of 0.5, most usages of this hook are obsolete.  
-        For a generic "object has been newly created from a row" hook, 
-        use ``on_reconstitute()``, or the @attributes.on_reconstitute 
+        unloaded attributes to be populated.  The method may be called
+        many times for a single instance, as multiple result rows are
+        used to populate eagerly loaded collections.
+
+        If this method returns EXT_CONTINUE, instance population will
+        proceed normally.  If any other value or None is returned,
+        instance population will not proceed, giving this extension an
+        opportunity to populate the instance itself, if desired.
+
+        As of 0.5, most usages of this hook are obsolete.  For a
+        generic "object has been newly created from a row" hook, use
+        ``reconstruct_instance()``, or the ``@orm.reconstructor``
         decorator.
-        
+
         """
         return EXT_CONTINUE
 
-    def on_reconstitute(self, mapper, instance):
-        """Receive an object instance after it has been created via 
-        ``__new__()``, and after initial attribute population has
-        occurred.  
-        
-        This typicically occurs when the instance is created based 
-        on incoming result rows, and is only called once for that
+    def reconstruct_instance(self, mapper, instance):
+        """Receive an object instance after it has been created via
+        ``__new__``, and after initial attribute population has
+        occurred.
+
+        This typicically occurs when the instance is created based on
+        incoming result rows, and is only called once for that
         instance's lifetime.
-        
+
         Note that during a result-row load, this method is called upon
-        the first row received for this instance; therefore, if eager loaders
-        are to further populate collections on the instance, those will
-        *not* have been completely loaded as of yet.
-        
+        the first row received for this instance. If eager loaders are
+        set to further populate collections on the instance, those
+        will *not* yet be completely loaded.
+
         """
         return EXT_CONTINUE
 
@@ -188,11 +187,12 @@ class MapperExtension(object):
         This is a good place to set up primary key values and such
         that aren't handled otherwise.
 
-        Column-based attributes can be modified within this method which will
-        result in the new value being inserted.  However *no* changes to the overall
-        flush plan can be made; this means any collection modification or
-        save() operations which occur within this method will not take effect
-        until the next flush call.
+        Column-based attributes can be modified within this method
+        which will result in the new value being inserted.  However
+        *no* changes to the overall flush plan can be made; this means
+        any collection modification or save() operations which occur
+        within this method will not take effect until the next flush
+        call.
 
         """
 
@@ -432,15 +432,15 @@ class MapperProperty(object):
 
 class PropComparator(expression.ColumnOperators):
     """defines comparison operations for MapperProperty objects.
-    
+
     PropComparator instances should also define an accessor 'property'
     which returns the MapperProperty associated with this
     PropComparator.
     """
-    
+
     def __clause_element__(self):
         raise NotImplementedError("%r" % self)
-        
+
     def contains_op(a, b):
         return a.contains(b)
     contains_op = staticmethod(contains_op)
@@ -456,30 +456,30 @@ class PropComparator(expression.ColumnOperators):
     def __init__(self, prop, mapper):
         self.prop = self.property = prop
         self.mapper = mapper
-        
+
     def of_type_op(a, class_):
         return a.of_type(class_)
     of_type_op = staticmethod(of_type_op)
-    
+
     def of_type(self, class_):
         """Redefine this object in terms of a polymorphic subclass.
-        
+
         Returns a new PropComparator from which further criterion can be evaluated.
 
         e.g.::
-        
+
             query.join(Company.employees.of_type(Engineer)).\\
                filter(Engineer.name=='foo')
-              
+
         \class_
             a class or mapper indicating that criterion will be against
             this specific subclass.
 
-         
+
         """
-        
+
         return self.operate(PropComparator.of_type_op, class_)
-        
+
     def contains(self, other):
         """Return true if this collection contains other"""
         return self.operate(PropComparator.contains_op, other)
@@ -531,18 +531,18 @@ class StrategizedProperty(MapperProperty):
                 return self.__init_strategy(cls)
         else:
             return self.strategy
-    
+
     def _get_strategy(self, cls):
         try:
             return self.__all_strategies[cls]
         except KeyError:
             return self.__init_strategy(cls)
-    
+
     def __init_strategy(self, cls):
         self.__all_strategies[cls] = strategy = cls(self)
         strategy.init()
         return strategy
-        
+
     def setup(self, context, entity, path, adapter, **kwargs):
         self.__get_context_strategy(context, path + (self.key,)).setup_query(context, entity, path, adapter, **kwargs)
 
@@ -631,10 +631,10 @@ class PropertyOption(MapperOption):
 
     def process_query_property(self, query, paths):
         pass
-    
+
     def __find_entity(self, query, mapper, raiseerr):
         from sqlalchemy.orm.util import _class_to_mapper, _is_aliased_class
-        
+
         if _is_aliased_class(mapper):
             searchfor = mapper
         else:
@@ -648,19 +648,19 @@ class PropertyOption(MapperOption):
                 raise sa_exc.ArgumentError("Can't find entity %s in Query.  Current list: %r" % (searchfor, [str(m.path_entity) for m in query._entities]))
             else:
                 return None
-            
+
     def __get_paths(self, query, raiseerr):
         path = None
         entity = None
         l = []
-        
+
         current_path = list(query._current_path)
-        
+
         if self.mapper:
             entity = self.__find_entity(query, self.mapper, raiseerr)
             mapper = entity.mapper
             path_element = entity.path_entity
-            
+
         for key in util.to_list(self.key):
             if isinstance(key, basestring):
                 tokens = key.split('.')
@@ -684,11 +684,11 @@ class PropertyOption(MapperOption):
                     key = prop.key
                 else:
                     raise sa_exc.ArgumentError("mapper option expects string key or list of attributes")
-            
+
                 if current_path and key == current_path[1]:
                     current_path = current_path[2:]
                     continue
-                
+
                 if prop is None:
                     return []
 
@@ -700,7 +700,7 @@ class PropertyOption(MapperOption):
                     path_element = mapper = getattr(prop, 'mapper', None)
                 if path_element:
                     path_element = path_element.base_mapper
-            
+
         return l
 
 PropertyOption.logger = log.class_logger(PropertyOption)
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 52acdcb33..ae356126e 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -14,6 +14,7 @@ available in [sqlalchemy.orm#].
 
 """
 
+import types
 import weakref
 from itertools import chain
 
@@ -106,15 +107,15 @@ class Mapper(object):
 
         self.class_ = class_
         self.class_manager = None
-            
+
         self.primary_key_argument = primary_key
         self.non_primary = non_primary
-        
+
         if order_by:
             self.order_by = util.to_list(order_by)
         else:
             self.order_by = order_by
-            
+
         self.always_refresh = always_refresh
         self.version_id_col = version_id_col
         self.concrete = concrete
@@ -135,7 +136,7 @@ class Mapper(object):
         self._clause_adapter = None
         self._requires_row_aliasing = False
         self.__inherits_equated_pairs = None
-        
+
         if not issubclass(class_, object):
             raise sa_exc.ArgumentError("Class '%s' is not a new-style class" % class_.__name__)
 
@@ -220,7 +221,7 @@ class Mapper(object):
 
     def has_property(self, key):
         return key in self.__props
-        
+
     def get_property(self, key, resolve_synonyms=False, raiseerr=True):
         """return a MapperProperty associated with the given key."""
 
@@ -347,12 +348,12 @@ class Mapper(object):
         global _new_mappers
         if self.compiled and not _new_mappers:
             return self
-            
+
         _COMPILE_MUTEX.acquire()
         global _already_compiling
         if _already_compiling:
-            # re-entrance to compile() occurs rarely, when a class-mapped construct is 
-            # used within a ForeignKey, something that is possible 
+            # re-entrance to compile() occurs rarely, when a class-mapped construct is
+            # used within a ForeignKey, something that is possible
             # when using the declarative layer
             self.__initialize_properties()
             return
@@ -367,7 +368,7 @@ class Mapper(object):
             for mapper in list(_mapper_registry):
                 if not mapper.compiled:
                     mapper.__initialize_properties()
-                    
+
             _new_mappers = False
             return self
         finally:
@@ -468,7 +469,7 @@ class Mapper(object):
 
             if self.order_by is False and not self.concrete and self.inherits.order_by is not False:
                 self.order_by = self.inherits.order_by
-                
+
             self.polymorphic_map = self.inherits.polymorphic_map
             self.batch = self.inherits.batch
             self.inherits._inheriting_mappers.add(self)
@@ -496,7 +497,7 @@ class Mapper(object):
                     raise sa_exc.ArgumentError("Mapper '%s' specifies a polymorphic_identity of '%s', but no mapper in it's hierarchy specifies the 'polymorphic_on' column argument" % (str(self), self.polymorphic_identity))
                 self.polymorphic_map[self.polymorphic_identity] = self
             self._identity_class = self.class_
-            
+
         if self.mapped_table is None:
             raise sa_exc.ArgumentError("Mapper '%s' does not have a mapped_table specified.  (Are you using the return value of table.create()?  It no longer has a return value.)" % str(self))
 
@@ -571,9 +572,9 @@ class Mapper(object):
         """Create a map of all *equivalent* columns, based on
         the determination of column pairs that are equated to
         one another based on inherit condition.  This is designed
-        to work with the queries that util.polymorphic_union 
+        to work with the queries that util.polymorphic_union
         comes up with, which often don't include the columns from
-        the base table directly (including the subclass table columns 
+        the base table directly (including the subclass table columns
         only).
 
         The resulting structure is a dictionary of columns mapped
@@ -638,15 +639,15 @@ class Mapper(object):
 
     def _should_exclude(self, name, local):
         """determine whether a particular property should be implicitly present on the class.
-        
-        This occurs when properties are propagated from an inherited class, or are 
+
+        This occurs when properties are propagated from an inherited class, or are
         applied from the columns present in the mapped table.
-        
+
         """
-        
+
         def is_userland_descriptor(obj):
             return not isinstance(obj, attributes.InstrumentedAttribute) and hasattr(obj, '__get__')
-            
+
         # check for descriptors, either local or from
         # an inherited class
         if local:
@@ -667,9 +668,9 @@ class Mapper(object):
             name in self.exclude_properties):
             self.__log("excluding property %s" % (name))
             return True
-            
+
         return False
-        
+
     def __compile_properties(self):
 
         # object attribute names mapped to MapperProperty objects
@@ -699,7 +700,7 @@ class Mapper(object):
 
             if self._should_exclude(column.key, local=self.local_table.c.contains_column(column)):
                 continue
-                
+
             column_key = (self.column_prefix or '') + column.key
 
             # adjust the "key" used for this column to that
@@ -707,7 +708,7 @@ class Mapper(object):
             for mapper in self.iterate_to_root():
                 if column in mapper._columntoproperty:
                     column_key = mapper._columntoproperty[column].key
-            
+
             self._compile_property(column_key, column, init=False, setparent=True)
 
         # do a special check for the "discriminiator" column, as it may only be present
@@ -762,13 +763,13 @@ class Mapper(object):
             # columns (included in zblog tests)
             if col is None:
                 col = prop.columns[0]
-                
+
                 # column is coming in after _readonly_props was initialized; check
                 # for 'readonly'
                 if hasattr(self, '_readonly_props') and \
                     (not hasattr(col, 'table') or col.table not in self._cols_by_table):
                         self._readonly_props.add(prop)
-                    
+
             else:
                 # if column is coming in after _cols_by_table was initialized, ensure the col is in the
                 # right set
@@ -792,14 +793,14 @@ class Mapper(object):
 
         self.__props[key] = prop
         prop.key = key
-        
+
         if setparent:
             prop.set_parent(self)
 
             if not self.non_primary:
                 self.class_manager.install_descriptor(
                     key, Mapper._CompileOnAttr(self.class_, key))
-                    
+
         if init:
             prop.init(key, self)
 
@@ -864,10 +865,16 @@ class Mapper(object):
         event_registry = manager.events
         event_registry.add_listener('on_init', _event_on_init)
         event_registry.add_listener('on_init_failure', _event_on_init_failure)
-        if 'on_reconstitute' in self.extension.methods:
-            def reconstitute(instance):
-                self.extension.on_reconstitute(self, instance)
-            event_registry.add_listener('on_load', reconstitute)
+        for key, method in util.iterate_attributes(self.class_):
+            if (isinstance(method, types.FunctionType) and
+                hasattr(method, '__sa_reconstructor__')):
+                event_registry.add_listener('on_load', method)
+                break
+
+        if 'reconstruct_instance' in self.extension.methods:
+            def reconstruct(instance):
+                self.extension.reconstruct_instance(self, instance)
+            event_registry.add_listener('on_load', reconstruct)
 
         manager.info[_INSTRUMENTOR] = self
 
@@ -1219,15 +1226,15 @@ class Mapper(object):
 
                     # testlib.pragma exempt:__hash__
                     inserted_objects.add((state, connection))
-        
+
         if not postupdate:
             for state, mapper, connection, has_identity in tups:
-                
+
                 # expire readonly attributes
                 readonly = state.unmodified.intersection(
                     p.key for p in mapper._readonly_props
                 )
-                
+
                 if readonly:
                     _expire_state(state, readonly)
 
@@ -1238,7 +1245,7 @@ class Mapper(object):
                     uowtransaction.session.query(self)._get(
                         state.key, refresh_state=state,
                         only_load_props=state.unloaded)
-                
+
                 # call after_XXX extensions
                 if not has_identity:
                     if 'after_insert' in mapper.extension.methods:
@@ -1253,10 +1260,10 @@ class Mapper(object):
     def __postfetch(self, uowtransaction, connection, table, state, resultproxy, params, value_params):
         """For a given Table that has just been inserted/updated,
         mark as 'expired' those attributes which correspond to columns
-        that are marked as 'postfetch', and populate attributes which 
+        that are marked as 'postfetch', and populate attributes which
         correspond to columns marked as 'prefetch' or were otherwise generated
         within _save_obj().
-        
+
         """
         postfetch_cols = resultproxy.postfetch_cols()
         generated_cols = list(resultproxy.prefetch_cols())
@@ -1274,7 +1281,7 @@ class Mapper(object):
                 self._set_state_attr_by_column(state, c, params[c.key])
 
         deferred_props = [prop.key for prop in [self._columntoproperty[c] for c in postfetch_cols]]
-        
+
         if deferred_props:
             _expire_state(state, deferred_props)
 
@@ -1462,7 +1469,7 @@ class Mapper(object):
                     identitykey = self._identity_key_from_state(refresh_state)
             else:
                 identitykey = identity_key(row)
-            
+
             if identitykey in session_identity_map:
                 instance = session_identity_map[identitykey]
                 state = attributes.instance_state(instance)
@@ -1538,7 +1545,7 @@ class Mapper(object):
                 # populate attributes on non-loading instances which have been expired
                 # TODO: apply eager loads to un-lazy loaded collections ?
                 if state in context.partials or state.unloaded:
-                        
+
                     if state in context.partials:
                         isnew = False
                         attrs = context.partials[state]
@@ -1588,7 +1595,7 @@ class Mapper(object):
 
         class ColumnsNotAvailable(Exception):
             pass
-            
+
         def visit_binary(binary):
             leftcol = binary.left
             rightcol = binary.right
@@ -1617,13 +1624,33 @@ class Mapper(object):
                     allconds.append(visitors.cloned_traverse(mapper.inherit_condition, {}, {'binary':visit_binary}))
         except ColumnsNotAvailable:
             return None
-            
+
         cond = sql.and_(*allconds)
         return sql.select(tables, cond, use_labels=True)
 
 Mapper.logger = log.class_logger(Mapper)
 
 
+def reconstructor(fn):
+    """Decorate a method as the 'reconstructor' hook.
+
+    Designates a method as the "reconstructor", an ``__init__``-like
+    method that will be called by the ORM after the instance has been
+    loaded from the database or otherwise reconstituted.
+
+    The reconstructor will be invoked with no arguments.  Scalar
+    (non-collection) database-mapped attributes of the instance will
+    be available for use within the function.  Eagerly-loaded
+    collections are generally not yet available and will usually only
+    contain the first element.  ORM state changes made to objects at
+    this stage will not be recorded for the next flush() operation, so
+    the activity within a reconstructor should be conservative.
+
+    """
+    fn.__sa_reconstructor__ = True
+    return fn
+
+
 def _event_on_init(state, instance, args, kwargs):
     """Trigger mapper compilation and run init_instance hooks."""
 
@@ -1654,7 +1681,7 @@ def _load_scalar_attributes(state, attribute_names):
         raise sa_exc.UnboundExecutionError("Instance %s is not bound to a Session; attribute refresh operation cannot proceed" % (state_str(state)))
 
     has_key = _state_has_identity(state)
-    
+
     result = False
     if mapper.inherits and not mapper.concrete:
         statement = mapper._optimized_get_statement(state, attribute_names)