- fix to subtle condition in topological sort where a node could appear twice,

for [ticket:362]

diff --git a/CHANGES b/CHANGES
index 058804bcfe2d26fe7149da955d8755e02ec3cf26..4b5b2e5d23502a161b66ae66a2a6ca7828b759d7 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -26,6 +26,8 @@ that the class of object attached to a parent object is appropriate
 mappings.  new example examples/association/proxied_association.py illustrates.
 - improvement to single table inheritance to load full hierarchies beneath 
 the target class
+- fix to subtle condition in topological sort where a node could appear twice,
+for [ticket:362]
 
 0.3.0
 - General:

diff --git a/lib/sqlalchemy/topological.py b/lib/sqlalchemy/topological.py
index 2a9d027e7a8bf2f87e1db87f3cf68c44a5eb8eab..9a7f2dd19214cbf83665bc1e0892c96c3b56fca6 100644 (file)
--- a/lib/sqlalchemy/topological.py
+++ b/lib/sqlalchemy/topological.py
@@ -133,8 +133,9 @@ class QueueDependencySorter(object):
                         lead.cycles.add(edge[1])
                         if n is not None:
                             queue.append(n)
-                            if n is not lead:
-                                n._cyclical = True
+                    for n in lead.cycles:
+                        if n is not lead:
+                            n._cyclical = True
                     # loop through cycle
                     # remove edges from the edge dictionary
                     # install the cycled nodes in the "cycle" list of one of the nodes

diff --git a/test/base/dependency.py b/test/base/dependency.py
index 88aacfe216515d33c493ec347d3a528265f698d0..0d278ebd1c7368ef4ef3941c3b69604be08731d5 100644 (file)
--- a/test/base/dependency.py
+++ b/test/base/dependency.py
@@ -17,6 +17,24 @@ class thingy(object):
         return repr(self)
         
 class DependencySortTest(PersistTest):
+    def assert_sort(self, tuples, node):
+        def assert_tuple(tuple, node):
+            if node.cycles:
+                cycles = [i.item for i in node.cycles]
+            else:
+                cycles = []
+            if tuple[0] is node.item or tuple[0] in cycles:
+                tuple.pop()
+                if tuple[0] is node.item or tuple[0] in cycles:
+                    return
+            elif len(tuple) > 1 and tuple[1] is node.item:
+                assert False, "Tuple not in dependency tree: " + str(tuple)
+            for c in node.children:
+                assert_tuple(tuple, c)
+        
+        for tuple in tuples:
+            assert_tuple(list(tuple), node)
+                    
     def testsort(self):
         rootnode = thingy('root')
         node2 = thingy('node2')
@@ -38,6 +56,7 @@ class DependencySortTest(PersistTest):
             (node4, subnode4)
         ]
         head = DependencySorter(tuples, []).sort()
+        self.assert_sort(tuples, head)
         print "\n" + str(head)
 
     def testsort2(self):
@@ -56,6 +75,7 @@ class DependencySortTest(PersistTest):
             (node6, node2)
         ]
         head = DependencySorter(tuples, [node7]).sort()
+        self.assert_sort(tuples, head)
         print "\n" + str(head)
 
     def testsort3(self):
@@ -90,6 +110,7 @@ class DependencySortTest(PersistTest):
             (node3, node2)
         ]
         head = DependencySorter(tuples, []).sort()
+        self.assert_sort(tuples, head)
         print "\n" + str(head)
 
     def testsort5(self):
@@ -118,6 +139,7 @@ class DependencySortTest(PersistTest):
             node4
         ]
         head = DependencySorter(tuples, allitems).sort()
+        self.assert_sort(tuples, head)
         print "\n" + str(head)
 
     def testcircular(self):
@@ -135,8 +157,26 @@ class DependencySortTest(PersistTest):
             (node4, node1)
         ]
         head = DependencySorter(tuples, []).sort(allow_all_cycles=True)
+        self.assert_sort(tuples, head)
+        print "\n" + str(head)
+        
+    def testcircular2(self):
+        # this condition was arising from ticket:362
+        # and was not treated properly by topological sort
+        node1 = thingy('node1')
+        node2 = thingy('node2')
+        node3 = thingy('node3')
+        node4 = thingy('node4')
+        tuples = [
+            (node1, node2),
+            (node3, node1),
+            (node2, node4),
+            (node3, node2),
+            (node2, node3)
+        ]
+        head = DependencySorter(tuples, []).sort(allow_all_cycles=True)
+        self.assert_sort(tuples, head)
         print "\n" + str(head)
         
-
 if __name__ == "__main__":
     unittest.main()