Fixed #23410 -- Avoided unnecessary rollbacks in related apps when migrating backwards. (ab2819aa) · Commits · Dom Sekotill / django

django/db/migrations/executor.py

+11 −6

Original line number	Diff line number	Diff line
		@@ -36,12 +36,17 @@ class MigrationExecutor(object):
		# If the migration is already applied, do backwards mode,
		# otherwise do forwards mode.
		elif target in applied:
		backwards_plan = self.loader.graph.backwards_plan(target)[:-1]
		# We only do this if the migration is not the most recent one
		# in its app - that is, another migration with the same app
		# label is in the backwards plan
		if any(node[0] == target[0] for node in backwards_plan):
		for migration in backwards_plan:
		# Don't migrate backwards all the way to the target node (that
		# may roll back dependencies in other apps that don't need to
		# be rolled back); instead roll back through target's immediate
		# child(ren) in the same app, and no further.
		next_in_app = sorted(
		n for n in
		self.loader.graph.dependents.get(target, set())
		if n[0] == target[0]
		)
		for node in next_in_app:
		for migration in self.loader.graph.backwards_plan(node):
		if migration in applied:
		plan.append((self.loader.graph.nodes[migration], True))
		applied.remove(migration)

docs/ref/django-admin.txt

+5 −4

Original line number	Diff line number	Diff line
		@@ -737,10 +737,11 @@ The behavior of this command changes depending on the arguments provided:
		* ``<app_label>``: The specified app has its migrations run, up to the most
		recent migration. This may involve running other apps' migrations too, due
		to dependencies.
		* ``<app_label> <migrationname>``: Brings the database schema to a state where it
		would have just run the given migration, but no further - this may involve
		unapplying migrations if you have previously migrated past the named
		migration. Use the name ``zero`` to unapply all migrations for an app.
		* ``<app_label> <migrationname>``: Brings the database schema to a state where
		the named migration is applied, but no later migrations in the same app are
		applied. This may involve unapplying migrations if you have previously
		migrated past the named migration. Use the name ``zero`` to unapply all
		migrations for an app.

		Unlike ``syncdb``, this command does not prompt you to create a superuser if
		one doesn't exist (assuming you are using :mod:`django.contrib.auth`). Use

docs/releases/1.7.2.txt

+3 −0

Original line number	Diff line number	Diff line
		@@ -68,3 +68,6 @@ Bugfixes

		* Made :class:`~django.db.migrations.operations.RenameModel` reversible
		(:ticket:`22248`)

		* Avoided unnecessary rollbacks of migrations from other apps when migrating
		backwards (:ticket:`23410`).

tests/migrations/test_executor.py

+95 −1

Original line number	Diff line number	Diff line
		from django.db import connection
		from django.db.migrations.executor import MigrationExecutor
		from django.test import modify_settings, override_settings
		from django.db.migrations.graph import MigrationGraph
		from django.test import modify_settings, override_settings, TestCase
		from django.apps.registry import apps as global_apps

		from .test_base import MigrationTestBase
		@@ -231,3 +232,96 @@ class ExecutorTests(MigrationTestBase):
		executor.migrate([("migrations", None)])
		self.assertTableNotExists("migrations_author")
		self.assertTableNotExists("migrations_tribble")


		class FakeLoader(object):
		def __init__(self, graph, applied):
		self.graph = graph
		self.applied_migrations = applied


		class FakeMigration(object):
		"""Really all we need is any object with a debug-useful repr."""
		def __init__(self, name):
		self.name = name

		def __repr__(self):
		return 'M<%s>' % self.name


		class ExecutorUnitTests(TestCase):
		"""(More) isolated unit tests for executor methods."""
		def test_minimize_rollbacks(self):
		"""
		Minimize unnecessary rollbacks in connected apps.

		When you say "./manage.py migrate appA 0001", rather than migrating to
		just after appA-0001 in the linearized migration plan (which could roll
		back migrations in other apps that depend on appA 0001, but don't need
		to be rolled back since we're not rolling back appA 0001), we migrate
		to just before appA-0002.
		"""
		a1_impl = FakeMigration('a1')
		a1 = ('a', '1')
		a2_impl = FakeMigration('a2')
		a2 = ('a', '2')
		b1_impl = FakeMigration('b1')
		b1 = ('b', '1')
		graph = MigrationGraph()
		graph.add_node(a1, a1_impl)
		graph.add_node(a2, a2_impl)
		graph.add_node(b1, b1_impl)
		graph.add_dependency(None, b1, a1)
		graph.add_dependency(None, a2, a1)

		executor = MigrationExecutor(None)
		executor.loader = FakeLoader(graph, {a1, b1, a2})

		plan = executor.migration_plan({a1})

		self.assertEqual(plan, [(a2_impl, True)])

		def test_minimize_rollbacks_branchy(self):
		"""
		Minimize rollbacks when target has multiple in-app children.

		a: 1 <---- 3 <--\
		\ \- 2 <--- 4
		\ \
		b: \- 1 <--- 2
		"""
		a1_impl = FakeMigration('a1')
		a1 = ('a', '1')
		a2_impl = FakeMigration('a2')
		a2 = ('a', '2')
		a3_impl = FakeMigration('a3')
		a3 = ('a', '3')
		a4_impl = FakeMigration('a4')
		a4 = ('a', '4')
		b1_impl = FakeMigration('b1')
		b1 = ('b', '1')
		b2_impl = FakeMigration('b2')
		b2 = ('b', '2')
		graph = MigrationGraph()
		graph.add_node(a1, a1_impl)
		graph.add_node(a2, a2_impl)
		graph.add_node(a3, a3_impl)
		graph.add_node(a4, a4_impl)
		graph.add_node(b1, b1_impl)
		graph.add_node(b2, b2_impl)
		graph.add_dependency(None, a2, a1)
		graph.add_dependency(None, a3, a1)
		graph.add_dependency(None, a4, a2)
		graph.add_dependency(None, a4, a3)
		graph.add_dependency(None, b2, b1)
		graph.add_dependency(None, b1, a1)
		graph.add_dependency(None, b2, a2)

		executor = MigrationExecutor(None)
		executor.loader = FakeLoader(graph, {a1, b1, a2, b2, a3, a4})

		plan = executor.migration_plan({a1})

		should_be_rolled_back = [b2_impl, a4_impl, a2_impl, a3_impl]
		exp = [(m, True) for m in should_be_rolled_back]
		self.assertEqual(plan, exp)