a simplification to syncrule generation, which also allows more flexible configuration

of which columns are to be involved in the synchronization via foreignkey property.
foreignkey param is a little more important now and should have its role clarified
particularly for self-referential mappers.

3 files changed, 49 insertions, 44 deletions

diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index 7bb550b4d..898d03eb6 100644
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -35,6 +35,7 @@ class DependencyProcessor(object):
         self.direction = prop.direction
         self.is_backref = prop.is_backref
         self.post_update = prop.post_update
+        self.foreignkey = prop.foreignkey
         self.key = prop.key
 
         self._compile_synchronizers()
@@ -84,15 +85,12 @@ class DependencyProcessor(object):
 
         The list of rules is used within commits by the _synchronize() method when dependent 
         objects are processed."""
-        parent_tables = util.Set(self.parent.tables + [self.parent.mapped_table])
-        target_tables = util.Set(self.mapper.tables + [self.mapper.mapped_table])
-
         self.syncrules = sync.ClauseSynchronizer(self.parent, self.mapper, self.direction)
         if self.direction == sync.MANYTOMANY:
-            self.syncrules.compile(self.prop.primaryjoin, parent_tables, [self.secondary], False)
-            self.syncrules.compile(self.prop.secondaryjoin, target_tables, [self.secondary], True)
+            self.syncrules.compile(self.prop.primaryjoin, issecondary=False)
+            self.syncrules.compile(self.prop.secondaryjoin, issecondary=True)
         else:
-            self.syncrules.compile(self.prop.primaryjoin, parent_tables, target_tables)
+            self.syncrules.compile(self.prop.primaryjoin, foreignkey=self.foreignkey)
         
     def get_object_dependencies(self, obj, uowcommit, passive = True):
         """returns the list of objects that are dependent on the given object, as according to the relationship
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 096e1ca33..1f4a8391d 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -271,7 +271,7 @@ class Mapper(object):
                     # stricter set of tables to create "sync rules" by,based on the immediate
                     # inherited table, rather than all inherited tables
                     self._synchronizer = sync.ClauseSynchronizer(self, self, sync.ONETOMANY)
-                    self._synchronizer.compile(self.mapped_table.onclause, util.Set([self.inherits.local_table]), sqlutil.TableFinder(self.local_table))
+                    self._synchronizer.compile(self.mapped_table.onclause)
             else:
                 self._synchronizer = None
                 self.mapped_table = self.local_table
diff --git a/lib/sqlalchemy/orm/sync.py b/lib/sqlalchemy/orm/sync.py
index aeaa5d1d2..5f0331e16 100644
--- a/lib/sqlalchemy/orm/sync.py
+++ b/lib/sqlalchemy/orm/sync.py
@@ -33,51 +33,57 @@ class ClauseSynchronizer(object):
         self.direction = direction
         self.syncrules = []
 
-    def compile(self, sqlclause, source_tables, target_tables, issecondary=None):
-        def check_for_table(binary, list1, list2):
-            #print "check for table", str(binary), [str(c) for c in l]
-            if binary.left.table in list1 and binary.right.table in list2:
-                return (binary.left, binary.right)
-            elif binary.right.table in list1 and binary.left.table in list2:
-                return (binary.right, binary.left)
-            else:
-                return (None, None)
-                
+    def compile(self, sqlclause, issecondary=None, foreignkey=None):
         def compile_binary(binary):
-            """assembles a SyncRule given a single binary condition"""
+            """assemble a SyncRule given a single binary condition"""
             if binary.operator != '=' or not isinstance(binary.left, schema.Column) or not isinstance(binary.right, schema.Column):
                 return
 
-            if binary.left.table == binary.right.table:
-                # self-cyclical relation
-                if binary.left.primary_key:
-                    source = binary.left
-                    dest = binary.right
-                elif binary.right.primary_key:
-                    source = binary.right
-                    dest = binary.left
+            source_column = None
+            dest_column = None
+            if foreignkey is not None:
+                # for self-referential relationships,
+                # the best we can do right now is figure out which side
+                # is the primary key
+                # TODO: need some better way for this
+                if binary.left.table == binary.right.table:
+                    if binary.left.primary_key:
+                        source_column = binary.left
+                        dest_column = binary.right
+                    elif binary.right.primary_key:
+                        source_column = binary.right
+                        dest_column = binary.left
+                    else:
+                        raise ArgumentError("Can't locate a primary key column in self-referential equality clause '%s'" % str(binary))
+                # for other relationships we are more flexible
+                # and go off the 'foreignkey' property
+                elif binary.left in foreignkey:
+                    dest_column = binary.left
+                    source_column = binary.right
+                elif binary.right in foreignkey:
+                    dest_column = binary.right
+                    source_column = binary.left
                 else:
-                    raise ArgumentError("Cant determine direction for relationship %s = %s" % (binary.left.table.fullname, binary.right.table.fullname))
+                    return
+            else:
+                if binary.left in [f.column for f in binary.right.foreign_keys]:
+                    dest_column = binary.right
+                    source_column = binary.left
+                elif binary.right in [f.column for f in binary.left.foreign_keys]:
+                    dest_column = binary.left
+                    source_column = binary.right
+            
+            if source_column and dest_column:    
                 if self.direction == ONETOMANY:
-                    self.syncrules.append(SyncRule(self.parent_mapper, source, dest, dest_mapper=self.child_mapper))
+                    self.syncrules.append(SyncRule(self.parent_mapper, source_column, dest_column, dest_mapper=self.child_mapper))
                 elif self.direction == MANYTOONE:
-                    self.syncrules.append(SyncRule(self.child_mapper, source, dest, dest_mapper=self.parent_mapper))
+                    self.syncrules.append(SyncRule(self.child_mapper, source_column, dest_column, dest_mapper=self.parent_mapper))
                 else:
-                    raise AssertionError("assert failed")
-            else:
-                (pt, tt) = check_for_table(binary, source_tables, target_tables)
-                #print "OK", binary, [t.name for t in source_tables], [t.name for t in target_tables]
-                if pt and tt:
-                    if self.direction == ONETOMANY:
-                        self.syncrules.append(SyncRule(self.parent_mapper, pt, tt, dest_mapper=self.child_mapper))
-                    elif self.direction == MANYTOONE:
-                        self.syncrules.append(SyncRule(self.child_mapper, tt, pt, dest_mapper=self.parent_mapper))
+                    if not issecondary:
+                        self.syncrules.append(SyncRule(self.parent_mapper, source_column, dest_column, dest_mapper=self.child_mapper, issecondary=issecondary))
                     else:
-                        if not issecondary:
-                            self.syncrules.append(SyncRule(self.parent_mapper, pt, tt, dest_mapper=self.child_mapper, issecondary=issecondary))
-                        else:
-                            self.syncrules.append(SyncRule(self.child_mapper, pt, tt, dest_mapper=self.parent_mapper, issecondary=issecondary))
-                            
+                        self.syncrules.append(SyncRule(self.child_mapper, source_column, dest_column, dest_mapper=self.parent_mapper, issecondary=issecondary))
+
         rules_added = len(self.syncrules)
         processor = BinaryVisitor(compile_binary)
         sqlclause.accept_visitor(processor)
@@ -131,7 +137,8 @@ class SyncRule(object):
             dest[self.dest_column.key] = value
         else:
             if clearkeys and self.dest_primary_key():
-                return
+                raise exceptions.AssertionError("Dependency rule tried to blank-out a primary key column")
+                
             if logging.is_debug_enabled(self.logger):
                 self.logger.debug("execute() instances: %s(%s)->%s(%s) ('%s')" % (mapperutil.instance_str(source), str(self.source_column), mapperutil.instance_str(dest), str(self.dest_column), value))
             self.dest_mapper._setattrbycolumn(dest, self.dest_column, value)