diff --git a/django/db/migrations/graph.py b/django/db/migrations/graph.py index 56b78d9056..3e3befe77f 100644 --- a/django/db/migrations/graph.py +++ b/django/db/migrations/graph.py @@ -1,4 +1,5 @@ from __future__ import unicode_literals +from collections import deque from django.utils.datastructures import OrderedSet from django.db.migrations.state import ProjectState @@ -96,28 +97,41 @@ 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): """ Dynamic programming based depth first search, for finding dependencies. """ - visited = [] + self.ensure_not_cyclic(start, get_children) + visited = deque() visited.append(start) - path = [start] - stack = sorted(get_children(start)) + stack = deque(sorted(get_children(start))) while stack: - node = stack.pop(0) - - if node in path: - raise CircularDependencyError() - path.append(node) - - visited.insert(0, node) - children = sorted(get_children(node)) - - if not children: - path = [] - - stack = children + stack + node = stack.popleft() + visited.appendleft(node) + children = sorted(get_children(node), reverse=True) + # reverse sorting is needed because prepending using deque.extendleft + # also effectively reverses values + stack.extendleft(children) return list(OrderedSet(visited)) diff --git a/tests/migrations/test_graph.py b/tests/migrations/test_graph.py index 3fcdd12667..3e066ed1f3 100644 --- a/tests/migrations/test_graph.py +++ b/tests/migrations/test_graph.py @@ -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") @@ -188,3 +202,30 @@ class GraphTests(TestCase): msg = "Migration app_a.0002 dependencies reference nonexistent child node ('app_a', '0002')" with self.assertRaisesMessage(KeyError, msg): graph.add_dependency("app_a.0002", ("app_a", "0002"), ("app_a", "0001")) + + def test_infinite_loop(self): + """ + Tests a complex dependency graph: + + app_a: 0001 <- + \ + app_b: 0001 <- x 0002 <- + / \ + app_c: 0001<- <------------- x 0002 + + And apply sqashing on app_c. + """ + graph = MigrationGraph() + + graph.add_node(("app_a", "0001"), None) + graph.add_node(("app_b", "0001"), None) + graph.add_node(("app_b", "0002"), None) + graph.add_node(("app_c", "0001_squashed_0002"), None) + + graph.add_dependency("app_b.0001", ("app_b", "0001"), ("app_c", "0001_squashed_0002")) + graph.add_dependency("app_b.0002", ("app_b", "0002"), ("app_a", "0001")) + graph.add_dependency("app_b.0002", ("app_b", "0002"), ("app_b", "0001")) + graph.add_dependency("app_c.0001_squashed_0002", ("app_c", "0001_squashed_0002"), ("app_b", "0002")) + + with self.assertRaises(CircularDependencyError): + graph.forwards_plan(("app_c", "0001_squashed_0002"))