diff options
-rw-r--r-- | CHANGES | 7 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/attributes.py | 56 | ||||
-rw-r--r-- | test/orm/test_attributes.py | 66 |
3 files changed, 111 insertions, 18 deletions
@@ -471,6 +471,13 @@ are also present in 0.8. is combined with uselist=False. This is an exception raise in 0.8. + - [bug] Fixed bug whereby user error in related-object + assignment could cause recursion overflow if the + assignment triggered a backref of the same name + as a bi-directional attribute on the incorrect + class to the same target. An informative + error is raised now. + - sql - [bug] Fixed CTE bug whereby positional bound parameters present in the CTEs themselves diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 2e576b4d8..d26ee61c3 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -983,6 +983,14 @@ def backref_listeners(attribute, key, uselist): # use easily recognizable names for stack traces + parent_token = attribute.impl.parent_token + + def _acceptable_key_err(child_state, initiator): + raise ValueError( + "Object %s not associated with attribute of " + "type %s" % (orm_util.state_str(child_state), + manager_of_class(initiator.class_)[initiator.key])) + def emit_backref_from_scalar_set_event(state, child, oldchild, initiator): if oldchild is child: return child @@ -1001,35 +1009,47 @@ def backref_listeners(attribute, key, uselist): if child is not None: child_state, child_dict = instance_state(child),\ instance_dict(child) - child_state.manager[key].impl.append( - child_state, - child_dict, - state.obj(), - initiator, - passive=PASSIVE_NO_FETCH) + child_impl = child_state.manager[key].impl + if initiator.parent_token is not parent_token and \ + initiator.parent_token is not child_impl.parent_token: + _acceptable_key_err(state, initiator) + child_impl.append( + child_state, + child_dict, + state.obj(), + initiator, + passive=PASSIVE_NO_FETCH) return child def emit_backref_from_collection_append_event(state, child, initiator): child_state, child_dict = instance_state(child), \ instance_dict(child) - child_state.manager[key].impl.append( - child_state, - child_dict, - state.obj(), - initiator, - passive=PASSIVE_NO_FETCH) + child_impl = child_state.manager[key].impl + if initiator.parent_token is not parent_token and \ + initiator.parent_token is not child_impl.parent_token: + _acceptable_key_err(state, initiator) + child_impl.append( + child_state, + child_dict, + state.obj(), + initiator, + passive=PASSIVE_NO_FETCH) return child def emit_backref_from_collection_remove_event(state, child, initiator): if child is not None: child_state, child_dict = instance_state(child),\ instance_dict(child) - child_state.manager[key].impl.pop( - child_state, - child_dict, - state.obj(), - initiator, - passive=PASSIVE_NO_FETCH) + child_impl = child_state.manager[key].impl + # can't think of a path that would produce an initiator + # mismatch here, as it would require an existing collection + # mismatch. + child_impl.pop( + child_state, + child_dict, + state.obj(), + initiator, + passive=PASSIVE_NO_FETCH) if uselist: event.listen(attribute, "append", diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index 8a6e37419..546822f5d 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -1158,6 +1158,72 @@ class BackrefTest(fixtures.ORMTest): # and this condition changes. assert c1 in p1.children +class CyclicBackrefAssertionTest(fixtures.TestBase): + """test that infinite recursion due to incorrect backref assignments + is blocked. + + """ + def test_scalar_set_type_assertion(self): + A, B, C = self._scalar_fixture() + c1 = C() + b1 = B() + assert_raises_message( + ValueError, + "Object <B at .*> not associated with attribute of type C.a", + setattr, c1, 'a', b1 + ) + + def test_collection_append_type_assertion(self): + A, B, C = self._collection_fixture() + c1 = C() + b1 = B() + assert_raises_message( + ValueError, + "Object <B at .*> not associated with attribute of type C.a", + c1.a.append, b1 + ) + + def _scalar_fixture(self): + class A(object): + pass + class B(object): + pass + class C(object): + pass + instrumentation.register_class(A) + instrumentation.register_class(B) + instrumentation.register_class(C) + attributes.register_attribute(C, 'a', backref='c', useobject=True) + attributes.register_attribute(C, 'b', backref='c', useobject=True) + + attributes.register_attribute(A, 'c', backref='a', useobject=True, + uselist=True) + attributes.register_attribute(B, 'c', backref='b', useobject=True, + uselist=True) + + return A, B, C + + def _collection_fixture(self): + class A(object): + pass + class B(object): + pass + class C(object): + pass + instrumentation.register_class(A) + instrumentation.register_class(B) + instrumentation.register_class(C) + + attributes.register_attribute(C, 'a', backref='c', useobject=True, + uselist=True) + attributes.register_attribute(C, 'b', backref='c', useobject=True, + uselist=True) + + attributes.register_attribute(A, 'c', backref='a', useobject=True) + attributes.register_attribute(B, 'c', backref='b', useobject=True) + + return A, B, C + class PendingBackrefTest(fixtures.ORMTest): def setup(self): global Post, Blog, called, lazy_load |