From 68ee348d36c7df8ccdaf57c3ca6da03e905590bc Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 9 Mar 2009 01:20:29 +0000 Subject: [PATCH] - a forward and complementing backwards reference which are both of the same direction, i.e. ONETOMANY or MANYTOONE, is now detected, and an error message is raised. Saves crazy CircularDependencyErrors later on. --- CHANGES | 7 +++- lib/sqlalchemy/orm/properties.py | 4 +++ test/orm/mapper.py | 2 +- test/orm/relationships.py | 57 ++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 921fbec76b..14a9c16ab6 100644 --- a/CHANGES +++ b/CHANGES @@ -42,7 +42,12 @@ CHANGES - Added "post_configure_attribute" method to InstrumentationManager, so that the "listen_for_events.py" example works again. [ticket:1314] - + + - a forward and complementing backwards reference which are both + of the same direction, i.e. ONETOMANY or MANYTOONE, + is now detected, and an error message is raised. + Saves crazy CircularDependencyErrors later on. + - Fixed bugs in Query regarding simultaneous selection of multiple joined-table inheritance entities with common base classes: diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 9c2268e014..3ae0bfbf5e 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -703,6 +703,10 @@ class RelationProperty(StrategizedProperty): raise sa_exc.ArgumentError("reverse_property %r on relation %s references " "relation %s, which does not reference mapper %s" % (key, self, other, self.parent)) + if self.direction in (ONETOMANY, MANYTOONE) and self.direction == other.direction: + raise sa_exc.ArgumentError("%s and back-reference %s are both of the same direction %r." + " Did you mean to set remote_side on the many-to-one side ?" % (self, other, self.direction)) + def do_init(self): self._get_target() self._process_dependent_arguments() diff --git a/test/orm/mapper.py b/test/orm/mapper.py index c1d422ec0a..59ae025cb3 100644 --- a/test/orm/mapper.py +++ b/test/orm/mapper.py @@ -447,7 +447,7 @@ class MapperTest(_fixtures.FixtureTest): include_properties=('id', 'type', 'name')) e_m = mapper(Employee, inherits=p_m, polymorphic_identity='employee', properties={ - 'boss': relation(Manager, backref='peon') + 'boss': relation(Manager, backref=backref('peon', ), remote_side=t.c.id) }, exclude_properties=('vendor_id',)) diff --git a/test/orm/relationships.py b/test/orm/relationships.py index 9787216f73..88f132eae2 100644 --- a/test/orm/relationships.py +++ b/test/orm/relationships.py @@ -1519,7 +1519,64 @@ class ExplicitLocalRemoteTest(_base.MappedTest): mapper(T2, t2) self.assertRaises(sa.exc.ArgumentError, sa.orm.compile_mappers) +class InvalidRemoteSideTest(_base.MappedTest): + def define_tables(self, metadata): + Table('t1', metadata, + Column('id', Integer, primary_key=True), + Column('data', String(50)), + Column('t_id', Integer, ForeignKey('t1.id')) + ) + @testing.resolve_artifact_names + def setup_classes(self): + class T1(_base.ComparableEntity): + pass + + @testing.resolve_artifact_names + def test_o2m_backref(self): + mapper(T1, t1, properties={ + 't1s':relation(T1, backref='parent') + }) + + self.assertRaisesMessage(sa.exc.ArgumentError, "T1.t1s and back-reference T1.parent are " + "both of the same direction . Did you " + "mean to set remote_side on the many-to-one side ?", sa.orm.compile_mappers) + + @testing.resolve_artifact_names + def test_m2o_backref(self): + mapper(T1, t1, properties={ + 't1s':relation(T1, backref=backref('parent', remote_side=t1.c.id), remote_side=t1.c.id) + }) + + self.assertRaisesMessage(sa.exc.ArgumentError, "T1.t1s and back-reference T1.parent are " + "both of the same direction . Did you " + "mean to set remote_side on the many-to-one side ?", sa.orm.compile_mappers) + + @testing.resolve_artifact_names + def test_o2m_explicit(self): + mapper(T1, t1, properties={ + 't1s':relation(T1, back_populates='parent'), + 'parent':relation(T1, back_populates='t1s'), + }) + + # can't be sure of ordering here + self.assertRaisesMessage(sa.exc.ArgumentError, + "both of the same direction . Did you " + "mean to set remote_side on the many-to-one side ?", sa.orm.compile_mappers) + + @testing.resolve_artifact_names + def test_m2o_explicit(self): + mapper(T1, t1, properties={ + 't1s':relation(T1, back_populates='parent', remote_side=t1.c.id), + 'parent':relation(T1, back_populates='t1s', remote_side=t1.c.id) + }) + + # can't be sure of ordering here + self.assertRaisesMessage(sa.exc.ArgumentError, + "both of the same direction . Did you " + "mean to set remote_side on the many-to-one side ?", sa.orm.compile_mappers) + + class InvalidRelationEscalationTest(_base.MappedTest): def define_tables(self, metadata): -- 2.47.2