Fixed #23556 -- Raised a more meaningful error message when migrations refer to an unavailable node (85086c81) · Commits · Dom Sekotill / django

django/db/migrations/graph.py

+27 −7

Original line number	Diff line number	Diff line
		from __future__ import unicode_literals

		from django.utils.datastructures import OrderedSet
		from django.db.migrations.state import ProjectState
		from django.utils.datastructures import OrderedSet


		class MigrationGraph(object):
		@@ -37,12 +37,14 @@ class MigrationGraph(object):

		def add_dependency(self, migration, child, parent):
		if child not in self.nodes:
		raise KeyError(
		"Migration %s dependencies reference nonexistent child node %r" % (migration, child)
		raise NodeNotFoundError(
		"Migration %s dependencies reference nonexistent child node %r" % (migration, child),
		child
		)
		if parent not in self.nodes:
		raise KeyError(
		"Migration %s dependencies reference nonexistent parent node %r" % (migration, parent)
		raise NodeNotFoundError(
		"Migration %s dependencies reference nonexistent parent node %r" % (migration, parent),
		parent
		)
		self.dependencies.setdefault(child, set()).add(parent)
		self.dependents.setdefault(parent, set()).add(child)
		@@ -55,7 +57,7 @@ class MigrationGraph(object):
		a database.
		"""
		if node not in self.nodes:
		raise ValueError("Node %r not a valid node" % (node, ))
		raise NodeNotFoundError("Node %r not a valid node" % (node, ), node)
		return self.dfs(node, lambda x: self.dependencies.get(x, set()))

		def backwards_plan(self, node):
		@@ -66,7 +68,7 @@ class MigrationGraph(object):
		a database.
		"""
		if node not in self.nodes:
		raise ValueError("Node %r not a valid node" % (node, ))
		raise NodeNotFoundError("Node %r not a valid node" % (node, ), node)
		return self.dfs(node, lambda x: self.dependents.get(x, set()))

		def root_nodes(self, app=None):
		@@ -160,3 +162,21 @@ class CircularDependencyError(Exception):
		Raised when there's an impossible-to-resolve circular dependency.
		"""
		pass


		class NodeNotFoundError(LookupError):
		"""
		Raised when an attempt on a node is made that is not available in the graph.
		"""

		def __init__(self, message, node):
		self.message = message
		self.node = node

		def __str__(self):
		return self.message

		__unicode__ = __str__

		def __repr__(self):
		return "NodeNotFoundError(%r)" % self.node

django/db/migrations/loader.py

+56 −6

Original line number	Diff line number	Diff line
		@@ -6,7 +6,7 @@ import sys

		from django.apps import apps
		from django.db.migrations.recorder import MigrationRecorder
		from django.db.migrations.graph import MigrationGraph
		from django.db.migrations.graph import MigrationGraph, NodeNotFoundError
		from django.utils import six
		from django.conf import settings

		@@ -118,7 +118,7 @@ class MigrationLoader(object):
		self.unmigrated_apps.add(app_config.label)

		def get_migration(self, app_label, name_prefix):
		"Gets the migration exactly named, or raises KeyError"
		"Gets the migration exactly named, or raises `graph.NodeNotFoundError`"
		return self.graph.nodes[app_label, name_prefix]

		def get_migration_by_prefix(self, app_label, name_prefix):
		@@ -156,7 +156,7 @@ class MigrationLoader(object):
		try:
		if key[1] == "__first__":
		return list(self.graph.root_nodes(key[0]))[0]
		else:
		else: # "__latest__"
		return list(self.graph.leaf_nodes(key[0]))[0]
		except IndexError:
		if self.ignore_no_migrations:
		@@ -194,6 +194,12 @@ class MigrationLoader(object):
		for key, migration in normal.items():
		for parent in migration.dependencies:
		reverse_dependencies.setdefault(parent, set()).add(key)
		# Remeber the possible replacements to generate more meaningful error
		# messages
		reverse_replacements = {}
		for key, migration in replacing.items():
		for replaced in migration.replaces:
		reverse_replacements.setdefault(replaced, set()).add(key)
		# Carry out replacements if we can - that is, if all replaced migrations
		# are either unapplied or missing.
		for key, migration in replacing.items():
		@@ -225,6 +231,32 @@ class MigrationLoader(object):
		self.graph = MigrationGraph()
		for key, migration in normal.items():
		self.graph.add_node(key, migration)

		def _reraise_missing_dependency(migration, missing, exc):
		"""
		Checks if ``missing`` could have been replaced by any squash
		migration but wasn't because the the squash migration was partially
		applied before. In that case raise a more understandable exception.

		#23556
		"""
		if missing in reverse_replacements:
		candidates = reverse_replacements.get(missing, set())
		is_replaced = any(candidate in self.graph.nodes for candidate in candidates)
		if not is_replaced:
		tries = ', '.join('%s.%s' % c for c in candidates)
		exc_value = NodeNotFoundError(
		"Migration {0} depends on nonexistent node ('{1}', '{2}'). "
		"Django tried to replace migration {1}.{2} with any of [{3}] "
		"but wasn't able to because some of the replaced migrations "
		"are already applied.".format(
		migration, missing[0], missing[1], tries
		),
		missing)
		exc_value.__cause__ = exc
		six.reraise(NodeNotFoundError, exc_value, sys.exc_info()[2])
		raise exc

		# Add all internal dependencies first to ensure __first__ dependencies
		# find the correct root node.
		for key, migration in normal.items():
		@@ -232,7 +264,15 @@ class MigrationLoader(object):
		if parent[0] != key[0] or parent[1] == '__first__':
		# Ignore __first__ references to the same app (#22325)
		continue
		try:
		self.graph.add_dependency(migration, key, parent)
		except NodeNotFoundError as e:
		# Since we added "key" to the nodes before this implies
		# "parent" is not in there. To make the raised exception
		# more understandable we check if parent could have been
		# replaced but hasn't (eg partially applied squashed
		# migration)
		_reraise_missing_dependency(migration, parent, e)
		for key, migration in normal.items():
		for parent in migration.dependencies:
		if parent[0] == key[0]:
		@@ -240,11 +280,21 @@ class MigrationLoader(object):
		continue
		parent = self.check_key(parent, key[0])
		if parent is not None:
		try:
		self.graph.add_dependency(migration, key, parent)
		except NodeNotFoundError as e:
		# Since we added "key" to the nodes before this implies
		# "parent" is not in there.
		_reraise_missing_dependency(migration, parent, e)
		for child in migration.run_before:
		child = self.check_key(child, key[0])
		if child is not None:
		try:
		self.graph.add_dependency(migration, child, key)
		except NodeNotFoundError as e:
		# Since we added "key" to the nodes before this implies
		# "child" is not in there.
		_reraise_missing_dependency(migration, child, e)

		def detect_conflicts(self):
		"""

tests/migrations/test_graph.py

+5 −5

Original line number	Diff line number	Diff line
		from django.test import TestCase
		from django.db.migrations.graph import MigrationGraph, CircularDependencyError
		from django.db.migrations.graph import CircularDependencyError, MigrationGraph, NodeNotFoundError


		class GraphTests(TestCase):
		@@ -156,10 +156,10 @@ class GraphTests(TestCase):
		graph = MigrationGraph()
		message = "Node ('app_b', '0001') not a valid node"

		with self.assertRaisesMessage(ValueError, message):
		with self.assertRaisesMessage(NodeNotFoundError, message):
		graph.forwards_plan(("app_b", "0001"))

		with self.assertRaisesMessage(ValueError, message):
		with self.assertRaisesMessage(NodeNotFoundError, message):
		graph.backwards_plan(("app_b", "0001"))

		def test_missing_parent_nodes(self):
		@@ -175,7 +175,7 @@ class GraphTests(TestCase):
		graph.add_dependency("app_a.0003", ("app_a", "0003"), ("app_a", "0002"))
		graph.add_dependency("app_a.0002", ("app_a", "0002"), ("app_a", "0001"))
		msg = "Migration app_a.0001 dependencies reference nonexistent parent node ('app_b', '0002')"
		with self.assertRaisesMessage(KeyError, msg):
		with self.assertRaisesMessage(NodeNotFoundError, msg):
		graph.add_dependency("app_a.0001", ("app_a", "0001"), ("app_b", "0002"))

		def test_missing_child_nodes(self):
		@@ -186,5 +186,5 @@ class GraphTests(TestCase):
		graph = MigrationGraph()
		graph.add_node(("app_a", "0001"), None)
		msg = "Migration app_a.0002 dependencies reference nonexistent child node ('app_a', '0002')"
		with self.assertRaisesMessage(KeyError, msg):
		with self.assertRaisesMessage(NodeNotFoundError, msg):
		graph.add_dependency("app_a.0002", ("app_a", "0002"), ("app_a", "0001"))

tests/migrations/test_loader.py

+104 −0

Original line number	Diff line number	Diff line
		from __future__ import unicode_literals

		from unittest import skipIf

		from django.test import TestCase, override_settings
		from django.db import connection, connections
		from django.db.migrations.graph import NodeNotFoundError
		from django.db.migrations.loader import MigrationLoader, AmbiguityError
		from django.db.migrations.recorder import MigrationRecorder
		from django.test import modify_settings
		@@ -187,3 +190,104 @@ class LoaderTests(TestCase):
		2,
		)
		recorder.flush()

		@override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_squashed_complex"})
		def test_loading_squashed_complex(self):
		"Tests loading a complex set of squashed migrations"

		loader = MigrationLoader(connection)
		recorder = MigrationRecorder(connection)

		def num_nodes():
		plan = set(loader.graph.forwards_plan(('migrations', '7_auto')))
		return len(plan - loader.applied_migrations)

		# Empty database: use squashed migration
		loader.build_graph()
		self.assertEqual(num_nodes(), 5)

		# Starting at 1 or 2 should use the squashed migration too
		recorder.record_applied("migrations", "1_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 4)

		recorder.record_applied("migrations", "2_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 3)

		# However, starting at 3 to 5 cannot use the squashed migration
		recorder.record_applied("migrations", "3_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 4)

		recorder.record_applied("migrations", "4_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 3)

		# Starting at 5 to 7 we are passed the squashed migrations
		recorder.record_applied("migrations", "5_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 2)

		recorder.record_applied("migrations", "6_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 1)

		recorder.record_applied("migrations", "7_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 0)

		recorder.flush()

		@override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_squashed_erroneous"})
		def test_loading_squashed_erroneous(self):
		"Tests loading a complex but erroneous set of squashed migrations"

		loader = MigrationLoader(connection)
		recorder = MigrationRecorder(connection)

		def num_nodes():
		plan = set(loader.graph.forwards_plan(('migrations', '7_auto')))
		return len(plan - loader.applied_migrations)

		# Empty database: use squashed migration
		loader.build_graph()
		self.assertEqual(num_nodes(), 5)

		# Starting at 1 or 2 should use the squashed migration too
		recorder.record_applied("migrations", "1_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 4)

		recorder.record_applied("migrations", "2_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 3)

		# However, starting at 3 or 4 we'd need to use non-existing migrations
		msg = ("Migration migrations.6_auto depends on nonexistent node ('migrations', '5_auto'). "
		"Django tried to replace migration migrations.5_auto with any of "
		"[migrations.3_squashed_5] but wasn't able to because some of the replaced "
		"migrations are already applied.")

		recorder.record_applied("migrations", "3_auto")
		with self.assertRaisesMessage(NodeNotFoundError, msg):
		loader.build_graph()

		recorder.record_applied("migrations", "4_auto")
		with self.assertRaisesMessage(NodeNotFoundError, msg):
		loader.build_graph()

		# Starting at 5 to 7 we are passed the squashed migrations
		recorder.record_applied("migrations", "5_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 2)

		recorder.record_applied("migrations", "6_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 1)

		recorder.record_applied("migrations", "7_auto")
		loader.build_graph()
		self.assertEqual(num_nodes(), 0)

		recorder.flush()

tests/migrations/test_migrations_squashed_complex/1_auto.py

0 → 100644

+11 −0

Original line number	Diff line number	Diff line
		# -- coding: utf-8 --
		from __future__ import unicode_literals

		from django.db import migrations


		class Migration(migrations.Migration):

		operations = [
		migrations.RunPython(lambda apps, schema_editor: None)
		]