From 24ee97c6100988a2ef80bd18d70418eb76070e4d Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 24 Aug 2008 21:52:38 +0000 Subject: [PATCH] - 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] --- CHANGES | 4 ++++ lib/sqlalchemy/orm/attributes.py | 15 ++++++++---- lib/sqlalchemy/orm/session.py | 4 ++-- test/orm/attributes.py | 40 +++++++++++++++++++++++++++++++- test/orm/naturalpks.py | 19 ++++++++++++++- 5 files changed, 73 insertions(+), 9 deletions(-) diff --git a/CHANGES b/CHANGES index ec383e1827..79ed4436e5 100644 --- 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 6fd1342421..88e124e4b6 100644 --- 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 20bb85e80b..b46ae16d32 100644 --- 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 eddf48cfe1..4e77935d47 100644 --- 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 d046fada65..b581141da5 100644 --- 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): -- 2.47.3