Commit ff3d746e authored by Luke Plant's avatar Luke Plant
Browse files

Fixed bug in circular dependency algo for migration dependencies. 

Previous algo only worked if the first item was a part of the loop,
and you would get an infinite loop otherwise (see test).

To fix this, it was much easier to do a pre-pass.

A bonus is that you now get an error message that actually helps you debug
the dependency problem.
parent 056a3c6c

django/db/migrations/graph.py

+21 −10
Original line number Diff line number Diff line
@@ -99,29 +99,40 @@ class MigrationGraph(object): 
                leaves.add(node)

        return sorted(leaves)



    def ensure_not_cyclic(self, start, get_children):

        # Algo from GvR:

        # http://neopythonic.blogspot.co.uk/2009/01/detecting-cycles-in-directed-graph.html

        todo = set(self.nodes.keys())

        while todo:

            node = todo.pop()

            stack = [node]

            while stack:

                top = stack[-1]

                for node in get_children(top):

                    if node in stack:

                        cycle = stack[stack.index(node):]

                        raise CircularDependencyError(", ".join("%s.%s" % n for n in cycle))

                    if node in todo:

                        stack.append(node)

                        todo.remove(node)

                        break

                else:

                    node = stack.pop()



    def dfs(self, start, get_children):

        """

        Iterative depth first search, for finding dependencies.

        """

        self.ensure_not_cyclic(start, get_children)

        visited = deque()

        visited.append(start)

        path = [start]

        stack = deque(sorted(get_children(start)))

        while stack:

            node = stack.popleft()



            if node in path:

                raise CircularDependencyError()

            path.append(node)



            visited.appendleft(node)

            children = sorted(get_children(node), reverse=True)

            # reverse sorting is needed because prepending using deque.extendleft

            # also effectively reverses values



            if path[-1] == node:

                path.pop()



            stack.extendleft(children)



        return list(OrderedSet(visited))

tests/migrations/test_graph.py

+14 −0
Original line number Diff line number Diff line
@@ -134,6 +134,20 @@ class GraphTests(TestCase): 
            graph.forwards_plan, ("app_a", "0003"),

        )



    def test_circular_graph_2(self):

        graph = MigrationGraph()

        graph.add_node(('A', '0001'), None)

        graph.add_node(('C', '0001'), None)

        graph.add_node(('B', '0001'), None)

        graph.add_dependency('A.0001', ('A', '0001'),('B', '0001'))

        graph.add_dependency('B.0001', ('B', '0001'),('A', '0001'))

        graph.add_dependency('C.0001', ('C', '0001'),('B', '0001'))



        self.assertRaises(

            CircularDependencyError,

            graph.forwards_plan, ('C', '0001')

        )



    def test_dfs(self):

        graph = MigrationGraph()

        root = ("app_a", "1")