- de-cruftified backref configuration code, backrefs which step on existing

  properties now raise an error [ticket:832]

diff --git a/CHANGES b/CHANGES
index 926ff76f88a3dab7330867d212d1b02bf8279748..eb9b5c12e57695106dc5b77f6894d6c0ff924f35 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -18,6 +18,9 @@ CHANGES
 - Renamed the Dialect attribute 'preexecute_sequences' to
   'preexecute_pk_sequences'.  An attribute proxy is in place for out-of-tree
   dialects using the old name.
+
+- de-cruftified backref configuration code, backrefs which step on existing
+  properties now raise an error [ticket:832]
   
 0.4.0
 -----

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index bc4cfeca08f10c71840aef2fac43d3853d40bd55..6967b5e3505bf9091bcf24f3c904fe4640cea1cd 100644 (file)
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -152,7 +152,7 @@ class PropertyLoader(StrategizedProperty):
         self.comparator = PropertyLoader.Comparator(self)
         self.join_depth = join_depth
         self.strategy_class = strategy_class
-        
+
         if cascade is not None:
             self.cascade = mapperutil.CascadeOptions(cascade)
         else:
@@ -661,52 +661,42 @@ class PropertyLoader(StrategizedProperty):
 PropertyLoader.logger = logging.class_logger(PropertyLoader)
 
 class BackRef(object):
-    """Stores the name of a backreference property as well as options
-    to be used on the resulting PropertyLoader.
-    """
+    """Attached to a PropertyLoader to indicate a complementary reverse relationship.
+    
+    Can optionally create the complementing PropertyLoader if one does not exist already."""
 
-    def __init__(self, key, **kwargs):
+    def __init__(self, key, _prop=None, **kwargs):
         self.key = key
         self.kwargs = kwargs
+        self.prop = _prop
 
     def compile(self, prop):
-        """Called by the owning PropertyLoader to set up a
-        backreference on the PropertyLoader's mapper.
-        """
-
-        # try to set a LazyLoader on our mapper referencing the parent mapper
+        if self.prop:
+            return
+        
+        self.prop = prop
+        
         mapper = prop.mapper.primary_mapper()
-        if not mapper.get_property(self.key, raiseerr=False) is not None:
+        if mapper.get_property(self.key, raiseerr=False) is None:
             pj = self.kwargs.pop('primaryjoin', None)
             sj = self.kwargs.pop('secondaryjoin', None)
-            # the backref property is set on the primary mapper
+
             parent = prop.parent.primary_mapper()
             self.kwargs.setdefault('viewonly', prop.viewonly)
             self.kwargs.setdefault('post_update', prop.post_update)
+                
             relation = PropertyLoader(parent, prop.secondary, pj, sj,
-                                      backref=prop.key, is_backref=True,
+                                      backref=BackRef(prop.key, _prop=prop), 
+                                      is_backref=True,
                                       **self.kwargs)
+                                      
             mapper._compile_property(self.key, relation);
-        elif not isinstance(mapper.get_property(self.key), PropertyLoader):
-            raise exceptions.ArgumentError(
-                "Can't create backref '%s' on mapper '%s'; an incompatible "
-                "property of that name already exists" % (self.key, str(mapper)))
+
+            prop.reverse_property = mapper.get_property(self.key)
+            mapper.get_property(self.key).reverse_property = prop
+
         else:
-            # else set one of us as the "backreference"
-            parent = prop.parent.primary_mapper()
-            if parent.class_ is not mapper.get_property(self.key)._get_target_class():
-                raise exceptions.ArgumentError(
-                    "Backrefs do not match:  backref '%s' expects to connect to %s, "
-                    "but found a backref already connected to %s" %
-                    (self.key, str(parent.class_), str(mapper.get_property(self.key).mapper.class_)))
-            if not mapper.get_property(self.key).is_backref:
-                prop.is_backref=True
-                if not prop.viewonly:
-                    prop._dependency_processor.is_backref=True
-                    # reverse_property used by dependencies.ManyToManyDP to check
-                    # association table operations
-                    prop.reverse_property = mapper.get_property(self.key)
-                    mapper.get_property(self.key).reverse_property = prop
+            raise exceptions.ArgumentError("Error creating backref '%s' on relation '%s': property of that name exists on mapper '%s'" % (self.key, prop, mapper))
 
     def get_extension(self):
         """Return an attribute extension to use with this backreference."""

diff --git a/test/orm/assorted_eager.py b/test/orm/assorted_eager.py
index 1eae7a5454e0e4dcfd9e14efb14bfdbf3db5c6a9..dab3d5de54453b3df2ca562877f3ba82d172ff2b 100644 (file)
--- a/test/orm/assorted_eager.py
+++ b/test/orm/assorted_eager.py
@@ -457,7 +457,6 @@ class EagerTest6(ORMTest):
         ))
 
         mapper(Design, design, properties=dict(
-            parts=relation(Part, private=True, backref="design"),
             inheritedParts=relation(InheritedPart, private=True, backref="design"),
         ))
 
@@ -466,7 +465,9 @@ class EagerTest6(ORMTest):
         ))
 
         class_mapper(Design).add_property("type", relation(DesignType, lazy=False, backref="designs"))
-        class_mapper(Part).add_property("design", relation(Design, lazy=False, backref="parts"))
+        
+        class_mapper(Part).add_property("design", relation(Design, lazy=False, backref=backref("parts", cascade="all, delete-orphan")))
+        
         #Part.mapper.add_property("designType", relation(DesignType))
 
         d = Design()

diff --git a/test/orm/compile.py b/test/orm/compile.py
index efbe2d4c67fef501cb0b8690b6b08bf83c90d11e..28f33c2a815b398b9df1c668e6b12fd364d7b8ec 100644 (file)
--- a/test/orm/compile.py
+++ b/test/orm/compile.py
@@ -7,7 +7,7 @@ from testlib import *
 
 class CompileTest(AssertMixin):
     """test various mapper compilation scenarios"""
-    def tearDownAll(self):
+    def tearDown(self):
         clear_mappers()
         
     def testone(self):
@@ -119,7 +119,7 @@ class CompileTest(AssertMixin):
             class_mapper(Product).compile()
             assert False
         except exceptions.ArgumentError, e:
-            assert str(e).index("Backrefs do not match") > -1
+            assert str(e).index("Error creating backref ") > -1
 
     def testthree(self):
         metadata = MetaData(testbase.db)
@@ -158,5 +158,27 @@ class CompileTest(AssertMixin):
         finally:
             metadata.drop_all()
 
+    def testfour(self):
+        meta = MetaData()
+        
+        a = Table('a', meta, Column('id', Integer, primary_key=True))
+        b = Table('b', meta, Column('id', Integer, primary_key=True), Column('a_id', Integer, ForeignKey('a.id')))
+
+        class A(object):pass
+        class B(object):pass
+
+        mapper(A, a, properties={
+            'b':relation(B, backref='a')
+        })
+        mapper(B, b, properties={
+            'a':relation(A, backref='b')
+        })
+        
+        try:
+            compile_mappers()
+            assert False
+        except exceptions.ArgumentError, e:
+            assert str(e).index("Error creating backref") > -1
+
 if __name__ == '__main__':
     testbase.main()

diff --git a/test/orm/inheritance/magazine.py b/test/orm/inheritance/magazine.py
index a0bf2414852d83237eadc7cedb14542ec5a37a54..8c2771c812b50a3b15e81d93d8861d5fc7d444fd 100644 (file)
--- a/test/orm/inheritance/magazine.py
+++ b/test/orm/inheritance/magazine.py
@@ -130,12 +130,10 @@ def generate_round_trip_test(use_unions=False, use_joins=False):
         location_name_mapper = mapper(LocationName, location_name_table)
 
         location_mapper = mapper(Location, location_table, properties = {
-            'issue': relation(Issue, backref='locations'),
+            'issue': relation(Issue, backref=backref('locations', lazy=False, cascade="all, delete-orphan")),
             '_name': relation(LocationName),
         })
 
-        issue_mapper.add_property('locations', relation(Location, lazy=False, private=True, backref='issue'))
-
         page_size_mapper = mapper(PageSize, page_size_table)
 
         magazine_mapper = mapper(Magazine, magazine_table, properties = {