- Fixed bug whereby changing a primary key attribute on an

entity where the attribute's previous value had been expired
would produce an error upon flush(). [ticket:1151]

diff --git a/CHANGES b/CHANGES
index ec383e18270b109c2c29ae8907453ef5dd4465fe..79ed4436e5f56357eb1146f097d7c823652c1202 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -68,6 +68,10 @@ CHANGES
     - The 3-tuple of iterables returned by attributes.get_history()
       may now be a mix of lists and tuples.  (Previously members
       were always lists.)
+      
+    - Fixed bug whereby changing a primary key attribute on an 
+      entity where the attribute's previous value had been expired 
+      would produce an error upon flush(). [ticket:1151]
 
     - Session.delete() adds the given object to the session if 
       not already present.  This was a regression bug from 0.4

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 6fd1342421971dc089e3a77e3801a8b8bb1dd7b1..88e124e4b6aae8500cf6716cbaeb2f11017b17dc 100644 (file)
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -217,12 +217,17 @@ class AttributeImpl(object):
           to it via this attribute.
 
         extension
-          an AttributeExtension object which will receive
-          set/delete/append/remove/etc. events.
+          a single or list of AttributeExtension object(s) which will 
+          receive set/delete/append/remove/etc. events.
 
         compare_function
           a function that compares two values which are normally
           assignable to this attribute.
+        
+        active_history
+          indicates that get_history() should always return the "old" value,
+          even if it means executing a lazy callable upon attribute change.
+          This flag is set to True if any extensions are present.
 
         """
 
@@ -231,12 +236,12 @@ class AttributeImpl(object):
         self.callable_ = callable_
         self.class_manager = class_manager
         self.trackparent = trackparent
-        self.active_history = active_history
         if compare_function is None:
             self.is_equal = operator.eq
         else:
             self.is_equal = compare_function
         self.extensions = util.to_list(extension or [])
+        self.active_history = active_history or bool(self.extensions)
 
     def hasparent(self, state, optimistic=False):
         """Return the boolean value of a `hasparent` flag attached to the given item.
@@ -367,7 +372,7 @@ class ScalarAttributeImpl(AttributeImpl):
     def delete(self, state):
 
         # TODO: catch key errors, convert to attributeerror?
-        if self.active_history or self.extensions:
+        if self.active_history:
             old = self.get(state)
         else:
             old = state.dict.get(self.key, NO_VALUE)
@@ -388,7 +393,7 @@ class ScalarAttributeImpl(AttributeImpl):
         if initiator is self:
             return
 
-        if self.active_history or self.extensions:
+        if self.active_history:
             old = self.get(state)
         else:
             old = state.dict.get(self.key, NO_VALUE)

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 20bb85e80b80719d056f9ca0e21e00213f351b8e..b46ae16d3213ace6ccb72067907090645f6cf76f 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1220,15 +1220,15 @@ class Session(object):
             merged_state._run_on_load(merged)
         return merged
 
+    @classmethod
     def identity_key(cls, *args, **kwargs):
         return mapperutil.identity_key(*args, **kwargs)
-    identity_key = classmethod(identity_key)
 
+    @classmethod
     def object_session(cls, instance):
         """Return the ``Session`` to which an object belongs."""
 
         return object_session(instance)
-    object_session = classmethod(object_session)
 
     def _validate_persistent(self, state):
         if not self.identity_map.contains_state(state):

diff --git a/test/orm/attributes.py b/test/orm/attributes.py
index eddf48cfe1aff55b372dfbd8a8c154bc7dbee408..4e77935d47f5a5aa6778ff0fa5b923e77af0cf42 100644 (file)
--- a/test/orm/attributes.py
+++ b/test/orm/attributes.py
@@ -1147,7 +1147,7 @@ class HistoryTest(_base.ORMTest):
         attributes.register_attribute(Foo, 'bar', uselist=False, callable_=lazyload, useobject=False)
         lazy_load = "hi"
 
-        # with scalar non-object, the lazy callable is only executed on gets, not history
+        # with scalar non-object and active_history=False, the lazy callable is only executed on gets, not history
         # operations
 
         f = Foo()
@@ -1171,6 +1171,44 @@ class HistoryTest(_base.ORMTest):
         assert f.bar is None
         eq_(attributes.get_history(attributes.instance_state(f), 'bar'), ([None], (), ["hi"]))
 
+    def test_scalar_via_lazyload_with_active(self):
+        class Foo(_base.BasicEntity):
+            pass
+
+        lazy_load = None
+        def lazyload(instance):
+            def load():
+                return lazy_load
+            return load
+
+        attributes.register_class(Foo)
+        attributes.register_attribute(Foo, 'bar', uselist=False, callable_=lazyload, useobject=False, active_history=True)
+        lazy_load = "hi"
+
+        # active_history=True means the lazy callable is executed on set as well as get,
+        # causing the old value to appear in the history
+
+        f = Foo()
+        eq_(f.bar, "hi")
+        eq_(attributes.get_history(attributes.instance_state(f), 'bar'), ((), ["hi"], ()))
+
+        f = Foo()
+        f.bar = None
+        eq_(attributes.get_history(attributes.instance_state(f), 'bar'), ([None], (), ['hi']))
+
+        f = Foo()
+        f.bar = "there"
+        eq_(attributes.get_history(attributes.instance_state(f), 'bar'), (["there"], (), ['hi']))
+        f.bar = "hi"
+        eq_(attributes.get_history(attributes.instance_state(f), 'bar'), ((), ["hi"], ()))
+
+        f = Foo()
+        eq_(f.bar, "hi")
+        del f.bar
+        eq_(attributes.get_history(attributes.instance_state(f), 'bar'), ((), (), ["hi"]))
+        assert f.bar is None
+        eq_(attributes.get_history(attributes.instance_state(f), 'bar'), ([None], (), ["hi"]))
+
     def test_scalar_object_via_lazyload(self):
         class Foo(_base.BasicEntity):
             pass

diff --git a/test/orm/naturalpks.py b/test/orm/naturalpks.py
index d046fada650dd041da333e43e67c57054058363f..b581141da576bb5f006990bcf4f03ad5ede8b484 100644 (file)
--- a/test/orm/naturalpks.py
+++ b/test/orm/naturalpks.py
@@ -62,7 +62,7 @@ class NaturalPKTest(_base.MappedTest):
         self.assertEquals(User(username='ed', fullname='jack'), u1)
 
     @testing.resolve_artifact_names
-    def test_expiry(self):
+    def test_load_after_expire(self):
         mapper(User, users)
 
         sess = create_session()
@@ -84,6 +84,23 @@ class NaturalPKTest(_base.MappedTest):
         assert sess.query(User).get('jack') is None
         assert sess.query(User).get('ed').fullname == 'jack'
 
+    @testing.resolve_artifact_names
+    def test_flush_new_pk_after_expire(self):
+        mapper(User, users)
+        sess = create_session()
+        u1 = User(username='jack', fullname='jack')
+
+        sess.add(u1)
+        sess.flush()
+        assert sess.query(User).get('jack') is u1
+
+        sess.expire(u1)
+        u1.username = 'ed'
+        sess.flush()
+        sess.clear()
+        assert sess.query(User).get('ed').fullname == 'jack'
+        
+
     @testing.fails_on('mysql')
     @testing.fails_on('sqlite')
     def test_onetomany_passive(self):