- re-establish and test some behavior from previous versions, that

if a load() or refresh() event changes history (which...why...but anyway)
the state of the object is the same; currently it seems that history
gets reset but on a refresh, the object still goes into session.dirty
- simplify what we store in partials

diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index 3a29fd7773d49207bf1bcc3d055e39ea931f0767..e728946e35e3004e4f80ac700b2545a77e906df1 100644 (file)
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -28,8 +28,7 @@ def instances(query, cursor, context):
 
     context.runid = _new_runid()
 
-    filter_fns = [ent.filter_fn
-                  for ent in query._entities]
+    filter_fns = [ent.filter_fn for ent in query._entities]
     filtered = id in filter_fns
 
     single_entity = len(query._entities) == 1 and \
@@ -384,7 +383,7 @@ def instance_processor(mapper, context, result, path, adapter,
                 state.manager.dispatch.refresh(
                     state, context, only_load_props)
 
-            if populate_existing:
+            if populate_existing or state.modified:
                 if refresh_state and only_load_props:
                     state._commit(dict_, only_load_props)
                 else:
@@ -398,33 +397,35 @@ def instance_processor(mapper, context, result, path, adapter,
 
             if state in context.partials:
                 isnew = False
-                (d_, attrs) = context.partials[state]
+                to_load = context.partials[state]
                 for key, populator in existing_populators:
-                    if key not in attrs:
+                    if key not in to_load:
                         continue
                     populator(state, dict_, row)
             else:
                 isnew = True
-                attrs = unloaded
-                context.partials[state] = (dict_, attrs)
+                to_load = unloaded
+                context.partials[state] = to_load
+
                 if context.propagate_options:
                     state.load_options = context.propagate_options
                 if state.load_options:
                     state.load_path = load_path
 
                 for key, populator in new_populators:
-                    if key not in attrs:
+                    if key not in to_load:
                         continue
                     populator(state, dict_, row)
 
-                state._commit(dict_, attrs)
-
             for key, pop in eager_populators:
                 if key not in unloaded:
                     pop(state, dict_, row)
 
             if isnew and refresh_evt:
-                state.manager.dispatch.refresh(state, context, attrs)
+                state.manager.dispatch.refresh(state, context, to_load)
+
+            if isnew:
+                state._commit(dict_, to_load)
 
         return instance
     return _instance

diff --git a/test/orm/test_events.py b/test/orm/test_events.py
index 068d73b07cc966ae61feebd1b35567e4dff2c80f..e6efd6fb91f4cb440c616517c1061c62785a19c3 100644 (file)
--- a/test/orm/test_events.py
+++ b/test/orm/test_events.py
@@ -933,6 +933,50 @@ class RefreshTest(_fixtures.FixtureTest):
         sess.query(User).first()
         eq_(canary, [])
 
+    def test_changes_reset(self):
+        """test the contract of load/refresh such that history is reset.
+
+        This has never been an official contract but we are testing it
+        here to ensure it is maintained given the loading performance
+        enhancements.
+
+        """
+        User = self.classes.User
+
+        @event.listens_for(User, "load")
+        def canary1(obj, context):
+            obj.name = 'new name!'
+
+        @event.listens_for(User, "refresh")
+        def canary2(obj, context, props):
+            obj.name = 'refreshed name!'
+
+        sess = Session()
+        u1 = User(name='u1')
+        sess.add(u1)
+        sess.commit()
+        sess.close()
+
+        u1 = sess.query(User).first()
+        eq_(
+            attributes.get_history(u1, "name"),
+            ((), ['new name!'], ())
+        )
+        assert "name" not in attributes.instance_state(u1).committed_state
+        assert u1 not in sess.dirty
+
+        sess.expire(u1)
+        u1.id
+        eq_(
+            attributes.get_history(u1, "name"),
+            ((), ['refreshed name!'], ())
+        )
+        assert "name" not in attributes.instance_state(u1).committed_state
+        assert u1 in sess.dirty
+
+
+
+
     def test_repeated_rows(self):
         User = self.classes.User