- Query.UpdateDeleteTest.test_delete_fallback fails on mysql due to subquery in DELETE; not sure how to do this exact operation in MySQL

- added query_cls keyword argument to sessionmaker(); allows user-defined Query subclasses to be generated by query().
- added @attributes.on_reconstitute decorator, MapperExtension.on_reconstitute, both receieve 'on_load' attribute event allowing
non-__init__ dependent instance initialization routines.
- push memusage to the top to avoid pointless heisenbugs
- renamed '_foostate'/'_fooclass_manager' to '_sa_instance_state'/'_sa_class_manager'
- removed legacy instance ORM state accessors
- query._get() will use _remove_newly_deleted instead of expunge() on ObjectDeleted, so that transaction rollback
restores the previous state
- removed MapperExtension.get(); replaced by a user-defined Query subclass
- removed needless **kwargs from query.get()
- removed Session.get(cls, id); this is redundant against Session.query(cls).get(id)
- removed Query.load() and Session.load(); the use case for this method has never been clear, and the same functionality is available in more explicit ways

5 files changed, 77 insertions, 142 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 295535b6d..2dbef2497 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -991,7 +991,7 @@ class Events(object):
             fn(*args, **kwargs)
 
     def add_listener(self, event, listener):
-        # not thread safe... problem?
+        # not thread safe... problem?  mb: nope
         bucket = getattr(self, event)
         if bucket == ():
             setattr(self, event, [listener])
@@ -1006,8 +1006,8 @@ class Events(object):
 class ClassManager(dict):
     """tracks state information at the class level."""
 
-    MANAGER_ATTR = '_fooclass_manager'
-    STATE_ATTR = '_foostate'
+    MANAGER_ATTR = '_sa_class_manager'
+    STATE_ATTR = '_sa_instance_state'
 
     event_registry_factory = Events
     instance_state_factory = InstanceState
@@ -1028,6 +1028,10 @@ class ClassManager(dict):
         self._instantiable = False
         self.events = self.event_registry_factory()
 
+        for meth in class_.__dict__.values():
+            if 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)
@@ -1438,6 +1442,19 @@ 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 eb333dfd2..4cfc9462a 100644
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -72,32 +72,15 @@ class MapperExtension(object):
     def init_failed(self, mapper, class_, oldinit, instance, args, kwargs):
         return EXT_CONTINUE
 
-    def load(self, query, *args, **kwargs):
-        """Override the `load` method of the Query object.
-
-        The return value of this method is used as the result of
-        ``query.load()`` if the value is anything other than EXT_CONTINUE.
-        """
-
-        return EXT_CONTINUE
-
-    def get(self, query, *args, **kwargs):
-        """Override the `get` method of the Query object.
-
-        The return value of this method is used as the result of
-        ``query.get()`` if the value is anything other than EXT_CONTINUE.
-        """
-
-        return EXT_CONTINUE
-
     def translate_row(self, mapper, context, row):
         """Perform pre-processing on the given result row and return a
         new row instance.
 
-        This is called as the very first step in the ``_instance()``
-        method.
+        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
 
     def create_instance(self, mapper, selectcontext, row, class_):
@@ -121,8 +104,8 @@ class MapperExtension(object):
 
         return value
           A new object instance, or EXT_CONTINUE
-        """
 
+        """
         return EXT_CONTINUE
 
     def append_result(self, mapper, selectcontext, row, instance, result, **flags):
@@ -152,25 +135,50 @@ class MapperExtension(object):
 
         \**flags
           extra information about the row, same as criterion in
-          `create_row_processor()` method of [sqlalchemy.orm.interfaces#MapperProperty]
+          ``create_row_processor()`` method of [sqlalchemy.orm.interfaces#MapperProperty]
         """
 
         return EXT_CONTINUE
 
     def populate_instance(self, mapper, selectcontext, row, instance, **flags):
-        """Receive a newly-created instance before that instance has
+        """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.
 
-        The normal population of attributes is according to each
-        attribute's corresponding MapperProperty (which includes
-        column-based attributes as well as relationships to other
-        classes).  If this method returns EXT_CONTINUE, instance
+        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 
+        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
+        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.
+        
+        """
         return EXT_CONTINUE
 
     def before_insert(self, mapper, connection, instance):
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 79e96dcaf..3139ac507 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -332,8 +332,6 @@ class Mapper(object):
         if not self.non_primary and self.entity_name in mappers:
             del mappers[self.entity_name]
         if not mappers and manager.info.get(_INSTRUMENTOR, False):
-            for legacy in _legacy_descriptors.keys():
-                manager.uninstall_member(legacy)
             manager.events.remove_listener('on_init', _event_on_init)
             manager.events.remove_listener('on_init_failure',
                                            _event_on_init_failure)
@@ -815,9 +813,11 @@ 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, impl in _legacy_descriptors.items():
-            manager.install_member(key, impl)
 
         manager.info[_INSTRUMENTOR] = self
 
@@ -1445,7 +1445,8 @@ class Mapper(object):
                     if instance is EXT_CONTINUE:
                         instance = self.class_manager.new_instance()
                     else:
-                        manager = attributes.manager_for_cls(instance.__class__)
+                        # TODO: don't think theres coverage here
+                        manager = attributes.manager_of_class(instance.__class__)
                         # TODO: if manager is None, raise a friendly error about
                         # returning instances of unmapped types
                         manager.setup_instance(instance)
@@ -1488,12 +1489,12 @@ class Mapper(object):
                     if not populate_instance or extension.populate_instance(self, context, row, instance, only_load_props=attrs, instancekey=identitykey, isnew=isnew) is EXT_CONTINUE:
                         populate_state(state, row, isnew, attrs, instancekey=identitykey)
 
-            if result is not None and (not append_result or extension.append_result(self, context, row, instance, result, instancekey=identitykey, isnew=isnew) is EXT_CONTINUE):
-                result.append(instance)
-
             if loaded_instance:
                 state._run_on_load(instance)
 
+            if result is not None and (not append_result or extension.append_result(self, context, row, instance, result, instancekey=identitykey, isnew=isnew) is EXT_CONTINUE):
+                result.append(instance)
+
             return instance
         return _instance
 
@@ -1572,45 +1573,6 @@ def _event_on_init_failure(state, instance, args, kwargs):
             instrumenting_mapper, instrumenting_mapper.class_,
             state.manager.events.original_init, instance, args, kwargs)
 
-def _legacy_descriptors():
-    """Build compatibility descriptors mapping legacy to InstanceState.
-
-    These are slated for removal in 0.5.  They were never part of the
-    official public API but were suggested as temporary workarounds in a
-    number of mailing list posts.  Permanent and public solutions for those
-    needs should be available now.  Consult the applicable mailing list
-    threads for details.
-
-    """
-    def _instance_key(self):
-        state = attributes.instance_state(self)
-        if state.key is not None:
-            return state.key
-        else:
-            raise AttributeError("_instance_key")
-    _instance_key = util.deprecated(None, False)(_instance_key)
-    _instance_key = property(_instance_key)
-
-    def _sa_session_id(self):
-        state = attributes.instance_state(self)
-        if state.session_id is not None:
-            return state.session_id
-        else:
-            raise AttributeError("_sa_session_id")
-    _sa_session_id = util.deprecated(None, False)(_sa_session_id)
-    _sa_session_id = property(_sa_session_id)
-
-    def _entity_name(self):
-        state = attributes.instance_state(self)
-        if state.entity_name is attributes.NO_ENTITY_NAME:
-            return None
-        else:
-            return state.entity_name
-    _entity_name = util.deprecated(None, False)(_entity_name)
-    _entity_name = property(_entity_name)
-
-    return dict(locals())
-_legacy_descriptors = _legacy_descriptors()
 
 def _load_scalar_attributes(state, attribute_names):
     mapper = _state_mapper(state)
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 700d24d33..df775a177 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -397,7 +397,7 @@ class Query(object):
         self._yield_per = count
     yield_per = _generative()(yield_per)
 
-    def get(self, ident, **kwargs):
+    def get(self, ident):
         """Return an instance of the object based on the given identifier, or None if not found.
 
         The `ident` argument is a scalar or tuple of primary key column values
@@ -405,39 +405,12 @@ class Query(object):
 
         """
 
-        ret = self._extension_zero().get(self, ident, **kwargs)
-        if ret is not mapper.EXT_CONTINUE:
-            return ret
-
-        # convert composite types to individual args
-        if hasattr(ident, '__composite_values__'):
-            ident = ident.__composite_values__()
-
-        key = self._only_mapper_zero().identity_key_from_primary_key(ident)
-        return self._get(key, ident, **kwargs)
-
-    def load(self, ident, raiseerr=True, **kwargs):
-        """Return an instance of the object based on the given identifier.
-
-        If not found, raises an exception.  The method will **remove all
-        pending changes** to the object already existing in the Session.  The
-        `ident` argument is a scalar or tuple of primary key column values in
-        the order of the table def's primary key columns.
-
-        """
-        ret = self._extension_zero().load(self, ident, **kwargs)
-        if ret is not mapper.EXT_CONTINUE:
-            return ret
-
         # convert composite types to individual args
         if hasattr(ident, '__composite_values__'):
             ident = ident.__composite_values__()
 
         key = self._only_mapper_zero().identity_key_from_primary_key(ident)
-        instance = self.populate_existing()._get(key, ident, **kwargs)
-        if instance is None and raiseerr:
-            raise sa_exc.InvalidRequestError("No instance found for identity %s" % repr(ident))
-        return instance
+        return self._get(key, ident)
 
     def query_from_parent(cls, instance, property, **kwargs):
         """Return a new Query with criterion corresponding to a parent instance.
@@ -1148,8 +1121,7 @@ class Query(object):
                     try:
                         state()
                     except orm_exc.ObjectDeletedError:
-                        # TODO: should we expunge ?  if so, should we expunge here ? or in mapper._load_scalar_attributes ?
-                        self.session.expunge(instance)
+                        self.session._remove_newly_deleted(state)
                         return None
                 return instance
             except KeyError:
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 5d6326ac4..a2dbb8e11 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -145,6 +145,10 @@ def sessionmaker(bind=None, class_=None, autoflush=True, autocommit=False,
       called. This allows each database to roll back the entire transaction,
       before each transaction is committed.
 
+    query_cls
+      Class which should be used to create new Query objects, as returned
+      by the ``query()`` method.  Defaults to [sqlalchemy.orm.query#Query].
+      
     weak_identity_map
       When set to the default value of ``False``, a weak-referencing map is
       used; instances which are not externally referenced will be garbage
@@ -505,13 +509,13 @@ class Session(object):
     public_methods = (
         '__contains__', '__iter__', 'add', 'add_all', 'begin', 'begin_nested',
         'clear', 'close', 'commit', 'connection', 'delete', 'execute', 'expire',
-        'expire_all', 'expunge', 'flush', 'get', 'get_bind', 'is_modified',
-        'load', 'merge', 'query', 'refresh', 'rollback', 'save',
+        'expire_all', 'expunge', 'flush', 'get_bind', 'is_modified',
+        'merge', 'query', 'refresh', 'rollback', 'save',
         'save_or_update', 'scalar', 'update')
 
     def __init__(self, bind=None, autoflush=True, autoexpire=True,
                  autocommit=False, twophase=False, echo_uow=False,
-                 weak_identity_map=True, binds=None, extension=None):
+                 weak_identity_map=True, binds=None, extension=None, query_cls=query.Query):
         """Construct a new Session.
 
         Arguments to ``Session`` are described using the
@@ -536,7 +540,7 @@ class Session(object):
         self.autoexpire = autoexpire
         self.twophase = twophase
         self.extension = extension
-        self._query_cls = query.Query
+        self._query_cls = query_cls
         self._mapper_flush_opts = {}
 
         if binds is not None:
@@ -896,34 +900,6 @@ class Session(object):
         for state in states:
             state.commit_all()
 
-    def get(self, class_, ident, entity_name=None):
-        """Return the instance of class with ident or None.
-
-        The `ident` argument is a scalar or tuple of primary key column values
-        in the order of the table def's primary key columns.
-
-        The `entity_name` keyword argument may also be specified which further
-        qualifies the underlying Mapper used to perform the query.
-
-        """
-        return self.query(_class_to_mapper(class_, entity_name)).get(ident)
-
-        return self.query(class_, entity_name=entity_name).get(ident)
-
-    def load(self, class_, ident, entity_name=None):
-        """Reset and return the instance of class with ident or raise.
-
-        If not found, raises an exception.  The method will **remove all
-        pending changes** to the object already existing in the ``Session``.
-        The `ident` argument is a scalar or tuple of primary key columns in
-        the order of the table def's primary key columns.
-
-        The `entity_name` keyword argument may also be specified which further
-        qualifies the underlying ``Mapper`` used to perform the query.
-
-        """
-        return self.query(_class_to_mapper(class_, entity_name)).load(ident)
-
     def refresh(self, instance, attribute_names=None):
         """Refresh the attributes on the given instance.
 
@@ -1201,7 +1177,7 @@ class Session(object):
                 self._update_impl(merged_state)
                 new_instance = True
             else:
-                merged = self.get(mapper.class_, key[1])
+                merged = self.query(mapper.class_).get(key[1])
 
         if merged is None:
             merged = mapper.class_manager.new_instance()