From: Mike Bayer Date: Mon, 13 Sep 2010 06:09:38 +0000 (-0400) Subject: - Fixed recursion bug which could occur when moving X-Git-Tag: rel_0_6_5~69 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f5d2bb60a886ba0aa35a04eb6c096b7982643ca2;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Fixed recursion bug which could occur when moving an object from one reference to another, with backrefs involved, where the initiating parent was a subclass (with its own mapper) of the previous parent. --- diff --git a/CHANGES b/CHANGES index c91596a092..a39fac9f0b 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,12 @@ CHANGES 0.6.5 ===== - orm + - Fixed recursion bug which could occur when moving + an object from one reference to another, with + backrefs involved, where the initiating parent + was a subclass (with its own mapper) of the + previous parent. + - Added an assertion during flush which ensures that no NULL-holding identity keys were generated on "newly persistent" objects. diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 33069332d9..b56de5f05d 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -455,7 +455,7 @@ class ScalarAttributeImpl(AttributeImpl): self, state, dict_.get(self.key, NO_VALUE)) def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF): - if initiator is self: + if initiator and initiator.parent_token is self.parent_token: return if self.active_history: @@ -534,7 +534,7 @@ class MutableScalarAttributeImpl(ScalarAttributeImpl): state.mutable_dict.pop(self.key) def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF): - if initiator is self: + if initiator and initiator.parent_token is self.parent_token: return if self.extensions: @@ -596,7 +596,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): setter operation. """ - if initiator is self: + if initiator and initiator.parent_token is self.parent_token: return if self.active_history: @@ -622,7 +622,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): previous is not None and previous is not PASSIVE_NO_RESULT): self.sethasparent(instance_state(previous), False) - + for ext in self.extensions: value = ext.set(state, value, previous, initiator or self) @@ -726,7 +726,7 @@ class CollectionAttributeImpl(AttributeImpl): self.key, state, self.collection_factory) def append(self, state, dict_, value, initiator, passive=PASSIVE_OFF): - if initiator is self: + if initiator and initiator.parent_token is self.parent_token: return collection = self.get_collection(state, dict_, passive=passive) @@ -739,7 +739,7 @@ class CollectionAttributeImpl(AttributeImpl): collection.append_with_event(value, initiator) def remove(self, state, dict_, value, initiator, passive=PASSIVE_OFF): - if initiator is self: + if initiator and initiator.parent_token is self.parent_token: return collection = self.get_collection(state, state.dict, passive=passive) @@ -759,7 +759,7 @@ class CollectionAttributeImpl(AttributeImpl): setter operation. """ - if initiator is self: + if initiator and initiator.parent_token is self.parent_token: return self._set_iterable( diff --git a/lib/sqlalchemy/orm/collections.py b/lib/sqlalchemy/orm/collections.py index 0789d9626c..b523295232 100644 --- a/lib/sqlalchemy/orm/collections.py +++ b/lib/sqlalchemy/orm/collections.py @@ -587,7 +587,7 @@ class CollectionAdapter(object): def fire_append_event(self, item, initiator=None): """Notify that a entity has entered the collection. - Initiator is the InstrumentedAttribute that initiated the membership + Initiator is a token owned by the InstrumentedAttribute that initiated the membership mutation, and should be left as None unless you are passing along an initiator value from a chained operation. diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index d558380114..c5ddaca40b 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -112,7 +112,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl): def set(self, state, dict_, value, initiator, passive=attributes.PASSIVE_OFF): - if initiator is self: + if initiator and initiator.parent_token is self.parent_token: return self._set_iterable(state, dict_, value) diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index 3a8a320e35..742e9d8747 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -693,14 +693,18 @@ class UtilTest(_base.ORMTest): class BackrefTest(_base.ORMTest): - def test_manytomany(self): + def test_m2m(self): class Student(object):pass class Course(object):pass attributes.register_class(Student) attributes.register_class(Course) - attributes.register_attribute(Student, 'courses', uselist=True, extension=attributes.GenericBackrefExtension('students'), useobject=True) - attributes.register_attribute(Course, 'students', uselist=True, extension=attributes.GenericBackrefExtension('courses'), useobject=True) + attributes.register_attribute(Student, 'courses', uselist=True, + extension=attributes.GenericBackrefExtension('students' + ), useobject=True) + attributes.register_attribute(Course, 'students', uselist=True, + extension=attributes.GenericBackrefExtension('courses' + ), useobject=True) s = Student() c = Course() @@ -717,14 +721,18 @@ class BackrefTest(_base.ORMTest): s1.courses.remove(c) self.assert_(c.students == [s2,s3]) - def test_onetomany(self): + def test_o2m(self): class Post(object):pass class Blog(object):pass attributes.register_class(Post) attributes.register_class(Blog) - attributes.register_attribute(Post, 'blog', uselist=False, extension=attributes.GenericBackrefExtension('posts'), trackparent=True, useobject=True) - attributes.register_attribute(Blog, 'posts', uselist=True, extension=attributes.GenericBackrefExtension('blog'), trackparent=True, useobject=True) + attributes.register_attribute(Post, 'blog', uselist=False, + extension=attributes.GenericBackrefExtension('posts'), + trackparent=True, useobject=True) + attributes.register_attribute(Blog, 'posts', uselist=True, + extension=attributes.GenericBackrefExtension('blog'), + trackparent=True, useobject=True) b = Blog() (p1, p2, p3) = (Post(), Post(), Post()) b.posts.append(p1) @@ -748,13 +756,17 @@ class BackrefTest(_base.ORMTest): p5.blog = None del p5.blog - def test_onetoone(self): + def test_o2o(self): class Port(object):pass class Jack(object):pass attributes.register_class(Port) attributes.register_class(Jack) - attributes.register_attribute(Port, 'jack', uselist=False, extension=attributes.GenericBackrefExtension('port'), useobject=True) - attributes.register_attribute(Jack, 'port', uselist=False, extension=attributes.GenericBackrefExtension('jack'), useobject=True) + attributes.register_attribute(Port, 'jack', uselist=False, + extension=attributes.GenericBackrefExtension('port'), + useobject=True) + attributes.register_attribute(Jack, 'port', uselist=False, + extension=attributes.GenericBackrefExtension('jack'), + useobject=True) p = Port() j = Jack() p.jack = j @@ -764,6 +776,96 @@ class BackrefTest(_base.ORMTest): j.port = None self.assert_(p.jack is None) + def test_symmetric_o2o_inheritance(self): + """Test that backref 'initiator' catching goes against + a token that is global to all InstrumentedAttribute objects + within a particular class, not just the indvidual IA object + since we use distinct objects in an inheritance scenario. + + """ + class Parent(object): + pass + class Child(object): + pass + class SubChild(Child): + pass + + p_token = object() + c_token = object() + + attributes.register_class(Parent) + attributes.register_class(Child) + attributes.register_class(SubChild) + attributes.register_attribute(Parent, 'child', uselist=False, + extension=attributes.GenericBackrefExtension('parent'), + parent_token = p_token, + useobject=True) + attributes.register_attribute(Child, 'parent', uselist=False, + extension=attributes.GenericBackrefExtension('child'), + parent_token = c_token, + useobject=True) + attributes.register_attribute(SubChild, 'parent', + uselist=False, + extension=attributes.GenericBackrefExtension('child'), + parent_token = c_token, + useobject=True) + + p1 = Parent() + c1 = Child() + p1.child = c1 + + c2 = SubChild() + c2.parent = p1 + + def test_symmetric_o2m_inheritance(self): + class Parent(object): + pass + class SubParent(Parent): + pass + class Child(object): + pass + + p_token = object() + c_token = object() + + attributes.register_class(Parent) + attributes.register_class(SubParent) + attributes.register_class(Child) + attributes.register_attribute(Parent, 'children', uselist=True, + extension=attributes.GenericBackrefExtension('parent'), + parent_token = p_token, + useobject=True) + attributes.register_attribute(SubParent, 'children', uselist=True, + extension=attributes.GenericBackrefExtension('parent'), + parent_token = p_token, + useobject=True) + attributes.register_attribute(Child, 'parent', uselist=False, + extension=attributes.GenericBackrefExtension('children'), + parent_token = c_token, + useobject=True) + + p1 = Parent() + p2 = SubParent() + c1 = Child() + + p1.children.append(c1) + + assert c1.parent is p1 + assert c1 in p1.children + + p2.children.append(c1) + assert c1.parent is p2 + + # note its still in p1.children - + # the event model currently allows only + # one level deep. without the parent_token, + # it keeps going until a ValueError is raised + # and this condition changes. + assert c1 in p1.children + + + + class PendingBackrefTest(_base.ORMTest): def setup(self): global Post, Blog, called, lazy_load