- ensure no behavioral change in lazy callables / expire

vs. 0.9

3 files changed, 70 insertions, 42 deletions

diff --git a/lib/sqlalchemy/orm/instrumentation.py b/lib/sqlalchemy/orm/instrumentation.py
index 78a573cfd..cce13f356 100644
--- a/lib/sqlalchemy/orm/instrumentation.py
+++ b/lib/sqlalchemy/orm/instrumentation.py
@@ -110,6 +110,13 @@ class ClassManager(dict):
             attr.impl for attr in
             self.values() if attr.impl.accepts_scalar_loader])
 
+    @_memoized_key_collection
+    def _non_scalar_loader_keys(self):
+        return frozenset([
+            attr.key for attr in self.values()
+            if not attr.impl.accepts_scalar_loader
+        ])
+
     @util.memoized_property
     def mapper(self):
         # raises unless self.mapper has been assigned
diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index 32e11e841..51ed1c048 100644
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -424,6 +424,10 @@ class InstanceState(interfaces.InspectionAttr):
              if impl.expire_missing or impl.key in dict_]
         )
 
+        if 'callables' in self.__dict__:
+            for k in self.expired_attributes.intersection(self.callables):
+                del self.callables[k]
+
         for k in self.manager._collection_impl_keys.intersection(dict_):
             collection = dict_.pop(k)
             collection._sa_adapter.invalidated = True
@@ -436,10 +440,17 @@ class InstanceState(interfaces.InspectionAttr):
     def _expire_attributes(self, dict_, attribute_names):
         pending = self.__dict__.get('_pending_mutations', None)
 
+        if 'callables' in self.__dict__:
+            callables = self.callables
+        else:
+            callables = None
+
         for key in attribute_names:
             impl = self.manager[key].impl
             if impl.accepts_scalar_loader:
                 self.expired_attributes.add(key)
+                if callables and key in callables:
+                    del callables[key]
             old = dict_.pop(key, None)
             if impl.collection and old is not None:
                 impl._invalidate_collection(old)
@@ -566,6 +577,16 @@ class InstanceState(interfaces.InspectionAttr):
         self.expired_attributes.difference_update(
             set(keys).intersection(dict_))
 
+        # the per-keys commit removes object-level callables,
+        # while that of commit_all does not.  it's not clear
+        # if this behavior has a clear rationale, however tests do
+        # ensure this is what it does.
+        if 'callables' in self.__dict__:
+            for key in set(self.callables).\
+                    intersection(keys).\
+                    intersection(dict_):
+                del self.callables[key]
+
     def _commit_all(self, dict_, instance_dict=None):
         """commit all attributes unconditionally.
 
@@ -575,7 +596,8 @@ class InstanceState(interfaces.InspectionAttr):
          - all attributes are marked as "committed"
          - the "strong dirty reference" is removed
          - the "modified" flag is set to False
-         - any "expired" markers/callables for attributes loaded are removed.
+         - any "expired" markers for scalar attributes loaded are removed.
+         - lazy load callables for objects / collections *stay*
 
         Attributes marked as "expired" can potentially remain
         "expired" after this step if a value was not populated in state.dict.
diff --git a/test/orm/test_expire.py b/test/orm/test_expire.py
index 150a1cb27..63341abec 100644
--- a/test/orm/test_expire.py
+++ b/test/orm/test_expire.py
@@ -885,7 +885,6 @@ class ExpireTest(_fixtures.FixtureTest):
 
         users, User = self.tables.users, self.classes.User
 
-
         mapper(User, users)
 
         sess = create_session()
@@ -894,32 +893,30 @@ class ExpireTest(_fixtures.FixtureTest):
         # callable
         u1 = sess.query(User).options(defer(User.name)).first()
         assert isinstance(
-                    attributes.instance_state(u1).callables['name'],
-                    strategies.LoadDeferredColumns
-                )
+            attributes.instance_state(u1).callables['name'],
+            strategies.LoadDeferredColumns
+        )
 
         # expire the attr, it gets the InstanceState callable
         sess.expire(u1, ['name'])
-        assert isinstance(
-                    attributes.instance_state(u1).callables['name'],
-                    state.InstanceState
-                )
+        assert 'name' in attributes.instance_state(u1).expired_attributes
+        assert 'name' not in attributes.instance_state(u1).callables
 
         # load it, callable is gone
         u1.name
+        assert 'name' not in attributes.instance_state(u1).expired_attributes
         assert 'name' not in attributes.instance_state(u1).callables
 
         # same for expire all
         sess.expunge_all()
         u1 = sess.query(User).options(defer(User.name)).first()
         sess.expire(u1)
-        assert isinstance(
-                    attributes.instance_state(u1).callables['name'],
-                    state.InstanceState
-                )
+        assert 'name' in attributes.instance_state(u1).expired_attributes
+        assert 'name' not in attributes.instance_state(u1).callables
 
         # load over it.  everything normal.
         sess.query(User).first()
+        assert 'name' not in attributes.instance_state(u1).expired_attributes
         assert 'name' not in attributes.instance_state(u1).callables
 
         sess.expunge_all()
@@ -927,15 +924,15 @@ class ExpireTest(_fixtures.FixtureTest):
         # for non present, still expires the same way
         del u1.name
         sess.expire(u1)
-        assert 'name' in attributes.instance_state(u1).callables
+        assert 'name' in attributes.instance_state(u1).expired_attributes
+        assert 'name' not in attributes.instance_state(u1).callables
 
     def test_state_deferred_to_col(self):
         """Behavioral test to verify the current activity of loader callables."""
 
         users, User = self.tables.users, self.classes.User
 
-
-        mapper(User, users, properties={'name':deferred(users.c.name)})
+        mapper(User, users, properties={'name': deferred(users.c.name)})
 
         sess = create_session()
         u1 = sess.query(User).options(undefer(User.name)).first()
@@ -944,13 +941,12 @@ class ExpireTest(_fixtures.FixtureTest):
         # mass expire, the attribute was loaded,
         # the attribute gets the callable
         sess.expire(u1)
-        assert isinstance(
-                    attributes.instance_state(u1).callables['name'],
-                    state.InstanceState
-                )
+        assert 'name' in attributes.instance_state(u1).expired_attributes
+        assert 'name' not in attributes.instance_state(u1).callables
 
-        # load it, callable is gone
+        # load it
         u1.name
+        assert 'name' not in attributes.instance_state(u1).expired_attributes
         assert 'name' not in attributes.instance_state(u1).callables
 
         # mass expire, attribute was loaded but then deleted,
@@ -960,60 +956,63 @@ class ExpireTest(_fixtures.FixtureTest):
         u1 = sess.query(User).options(undefer(User.name)).first()
         del u1.name
         sess.expire(u1)
+        assert 'name' not in attributes.instance_state(u1).expired_attributes
         assert 'name' not in attributes.instance_state(u1).callables
 
         # single attribute expire, the attribute gets the callable
         sess.expunge_all()
         u1 = sess.query(User).options(undefer(User.name)).first()
         sess.expire(u1, ['name'])
-        assert isinstance(
-                    attributes.instance_state(u1).callables['name'],
-                    state.InstanceState
-                )
+        assert 'name' in attributes.instance_state(u1).expired_attributes
+        assert 'name' not in attributes.instance_state(u1).callables
 
     def test_state_noload_to_lazy(self):
         """Behavioral test to verify the current activity of loader callables."""
 
-        users, Address, addresses, User = (self.tables.users,
-                                self.classes.Address,
-                                self.tables.addresses,
-                                self.classes.User)
-
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User)
 
-        mapper(User, users, properties={'addresses':relationship(Address, lazy='noload')})
+        mapper(
+            User, users,
+            properties={'addresses': relationship(Address, lazy='noload')})
         mapper(Address, addresses)
 
         sess = create_session()
         u1 = sess.query(User).options(lazyload(User.addresses)).first()
         assert isinstance(
-                    attributes.instance_state(u1).callables['addresses'],
-                    strategies.LoadLazyAttribute
-                )
+            attributes.instance_state(u1).callables['addresses'],
+            strategies.LoadLazyAttribute
+        )
         # expire, it stays
         sess.expire(u1)
+        assert 'addresses' not in attributes.instance_state(u1).expired_attributes
         assert isinstance(
-                    attributes.instance_state(u1).callables['addresses'],
-                    strategies.LoadLazyAttribute
-                )
+            attributes.instance_state(u1).callables['addresses'],
+            strategies.LoadLazyAttribute
+        )
 
         # load over it.  callable goes away.
         sess.query(User).first()
+        assert 'addresses' not in attributes.instance_state(u1).expired_attributes
         assert 'addresses' not in attributes.instance_state(u1).callables
 
         sess.expunge_all()
         u1 = sess.query(User).options(lazyload(User.addresses)).first()
         sess.expire(u1, ['addresses'])
+        assert 'addresses' not in attributes.instance_state(u1).expired_attributes
         assert isinstance(
-                    attributes.instance_state(u1).callables['addresses'],
-                    strategies.LoadLazyAttribute
-                )
+            attributes.instance_state(u1).callables['addresses'],
+            strategies.LoadLazyAttribute
+        )
 
         # load the attr, goes away
         u1.addresses
+        assert 'addresses' not in attributes.instance_state(u1).expired_attributes
         assert 'addresses' not in attributes.instance_state(u1).callables
 
-
-
 class PolymorphicExpireTest(fixtures.MappedTest):
     run_inserts = 'once'
     run_deletes = None