Fixed #22568: Better proxy model support in migrations (c1276785) · Commits · Dom Sekotill / django

django/db/migrations/autodetector.py

+103 −19

Original line number	Diff line number	Diff line
		@@ -3,6 +3,7 @@ from __future__ import unicode_literals
		import re
		import datetime

		from django.utils import six
		from django.db import models
		from django.db.migrations import operations
		from django.db.migrations.migration import Migration
		@@ -104,22 +105,31 @@ class MigrationAutodetector(object):
		# into migrations to resolve dependencies caused by M2Ms and FKs.
		self.generated_operations = {}

		# Prepare some old/new state and model lists, ignoring
		# proxy models and unmigrated apps.
		# Prepare some old/new state and model lists, separating
		# proxy models and ignoring unmigrated apps.
		self.old_apps = self.from_state.render(ignore_swappable=True)
		self.new_apps = self.to_state.render()
		self.old_model_keys = []
		self.old_proxy_keys = []
		self.new_model_keys = []
		self.new_proxy_keys = []
		for al, mn in sorted(self.from_state.models.keys()):
		model = self.old_apps.get_model(al, mn)
		if not model._meta.proxy and model._meta.managed and al not in self.from_state.real_apps:
		if model._meta.managed and al not in self.from_state.real_apps:
		if model._meta.proxy:
		self.old_proxy_keys.append((al, mn))
		else:
		self.old_model_keys.append((al, mn))
		self.new_model_keys = []

		for al, mn in sorted(self.to_state.models.keys()):
		model = self.new_apps.get_model(al, mn)
		if not model._meta.proxy and model._meta.managed and (
		if model._meta.managed and (
		al not in self.from_state.real_apps or
		(convert_apps and al in convert_apps)
		):
		if model._meta.proxy:
		self.new_proxy_keys.append((al, mn))
		else:
		self.new_model_keys.append((al, mn))

		# Renames have to come first
		@@ -155,6 +165,8 @@ class MigrationAutodetector(object):
		# Generate non-rename model operations
		self.generate_created_models()
		self.generate_deleted_models()
		self.generate_created_proxies()
		self.generate_deleted_proxies()
		self.generate_altered_options()

		# Generate field operations
		@@ -232,7 +244,12 @@ class MigrationAutodetector(object):
		if self.migrations.get(dep[0], None):
		operation_dependencies.add((dep[0], self.migrations[dep[0]][-1].name))
		else:
		# If we can't find the other app, we add a __latest__ dependency,
		# but only if we've already been through once and checked everything
		if chop_mode:
		operation_dependencies.add((dep[0], "__latest__"))
		else:
		deps_satisfied = False
		if deps_satisfied:
		chopped.append(operation)
		dependencies.update(operation_dependencies)
		@@ -306,6 +323,12 @@ class MigrationAutodetector(object):
		operation.model_name.lower() == dependency[1].lower() and
		operation.name.lower() == dependency[2].lower()
		)
		# Removed model
		elif dependency[2] is None and dependency[3] is False:
		return (
		isinstance(operation, operations.DeleteModel) and
		operation.name.lower() == dependency[1].lower()
		)
		# order_with_respect_to being unset for a field
		elif dependency[2] is not None and dependency[3] == "order_wrt_unset":
		return (
		@@ -384,7 +407,16 @@ class MigrationAutodetector(object):
		unique_together = model_state.options.pop('unique_together', None)
		index_together = model_state.options.pop('index_together', None)
		order_with_respect_to = model_state.options.pop('order_with_respect_to', None)
		# Generate creation operatoin
		# Depend on the deletion of any possible proxy version of us
		dependencies = [
		(app_label, model_name, None, False),
		]
		# Depend on all bases
		for base in model_state.bases:
		if isinstance(base, six.string_types) and "." in base:
		base_app_label, base_name = base.split(".", 1)
		dependencies.append((base_app_label, base_name, None, False))
		# Generate creation operation
		self.add_operation(
		app_label,
		operations.CreateModel(
		@@ -392,7 +424,8 @@ class MigrationAutodetector(object):
		fields=[d for d in model_state.fields if d[0] not in related_fields],
		options=model_state.options,
		bases=model_state.bases,
		)
		),
		dependencies = dependencies,
		)
		# Generate operations for each related field
		for name, field in sorted(related_fields.items()):
		@@ -412,6 +445,8 @@ class MigrationAutodetector(object):
		None,
		True
		))
		# Depend on our own model being created
		dependencies.append((app_label, model_name, None, True))
		# Make operation
		self.add_operation(
		app_label,
		@@ -423,6 +458,11 @@ class MigrationAutodetector(object):
		dependencies=list(set(dependencies)),
		)
		# Generate other opns
		related_dependencies = [
		(app_label, model_name, name, True)
		for name, field in sorted(related_fields.items())
		]
		related_dependencies.append((app_label, model_name, None, True))
		if unique_together:
		self.add_operation(
		app_label,
		@@ -430,10 +470,7 @@ class MigrationAutodetector(object):
		name=model_name,
		unique_together=unique_together,
		),
		dependencies=[
		(app_label, model_name, name, True)
		for name, field in sorted(related_fields.items())
		]
		dependencies=related_dependencies
		)
		if index_together:
		self.add_operation(
		@@ -442,10 +479,7 @@ class MigrationAutodetector(object):
		name=model_name,
		index_together=index_together,
		),
		dependencies=[
		(app_label, model_name, name, True)
		for name, field in sorted(related_fields.items())
		]
		dependencies=related_dependencies
		)
		if order_with_respect_to:
		self.add_operation(
		@@ -456,9 +490,43 @@ class MigrationAutodetector(object):
		),
		dependencies=[
		(app_label, model_name, order_with_respect_to, True),
		(app_label, model_name, None, True),
		]
		)

		def generate_created_proxies(self):
		"""
		Makes CreateModel statements for proxy models.
		We use the same statements as that way there's less code duplication,
		but of course for proxy models we can skip all that pointless field
		stuff and just chuck out an operation.
		"""
		added_proxies = set(self.new_proxy_keys) - set(self.old_proxy_keys)
		for app_label, model_name in sorted(added_proxies):
		model_state = self.to_state.models[app_label, model_name]
		assert model_state.options.get("proxy", False)
		# Depend on the deletion of any possible non-proxy version of us
		dependencies = [
		(app_label, model_name, None, False),
		]
		# Depend on all bases
		for base in model_state.bases:
		if isinstance(base, six.string_types) and "." in base:
		base_app_label, base_name = base.split(".", 1)
		dependencies.append((base_app_label, base_name, None, False))
		# Generate creation operation
		self.add_operation(
		app_label,
		operations.CreateModel(
		name=model_state.name,
		fields=[],
		options=model_state.options,
		bases=model_state.bases,
		),
		# Depend on the deletion of any possible non-proxy version of us
		dependencies = dependencies,
		)

		def generate_deleted_models(self):
		"""
		Find all deleted models and make creation operations for them,
		@@ -547,6 +615,21 @@ class MigrationAutodetector(object):
		dependencies=list(set(dependencies)),
		)

		def generate_deleted_proxies(self):
		"""
		Makes DeleteModel statements for proxy models.
		"""
		deleted_proxies = set(self.old_proxy_keys) - set(self.new_proxy_keys)
		for app_label, model_name in sorted(deleted_proxies):
		model_state = self.from_state.models[app_label, model_name]
		assert model_state.options.get("proxy", False)
		self.add_operation(
		app_label,
		operations.DeleteModel(
		name=model_state.name,
		),
		)

		def generate_added_fields(self):
		# New fields
		self.renamed_fields = {}
		@@ -687,7 +770,8 @@ class MigrationAutodetector(object):
		makes an operation to represent them in state changes (in case Python
		code in migrations needs them)
		"""
		for app_label, model_name in sorted(self.kept_model_keys):
		models_to_check = self.kept_model_keys.union(set(self.new_proxy_keys).intersection(self.old_proxy_keys))
		for app_label, model_name in sorted(models_to_check):
		old_model_name = self.renamed_models.get((app_label, model_name), model_name)
		old_model_state = self.from_state.models[app_label, old_model_name]
		new_model_state = self.to_state.models[app_label, model_name]

django/db/migrations/operations/models.py

+5 −5

Original line number	Diff line number	Diff line
		@@ -32,17 +32,17 @@ class CreateModel(Operation):
		def database_forwards(self, app_label, schema_editor, from_state, to_state):
		apps = to_state.render()
		model = apps.get_model(app_label, self.name)
		if router.allow_migrate(schema_editor.connection.alias, model):
		if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy:
		schema_editor.create_model(model)

		def database_backwards(self, app_label, schema_editor, from_state, to_state):
		apps = from_state.render()
		model = apps.get_model(app_label, self.name)
		if router.allow_migrate(schema_editor.connection.alias, model):
		if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy:
		schema_editor.delete_model(model)

		def describe(self):
		return "Create model %s" % (self.name, )
		return "Create %smodel %s" % ("proxy " if self.options.get("proxy", False) else "", self.name)

		def references_model(self, name, app_label=None):
		strings_to_check = [self.name]
		@@ -85,13 +85,13 @@ class DeleteModel(Operation):
		def database_forwards(self, app_label, schema_editor, from_state, to_state):
		apps = from_state.render()
		model = apps.get_model(app_label, self.name)
		if router.allow_migrate(schema_editor.connection.alias, model):
		if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy:
		schema_editor.delete_model(model)

		def database_backwards(self, app_label, schema_editor, from_state, to_state):
		apps = to_state.render()
		model = apps.get_model(app_label, self.name)
		if router.allow_migrate(schema_editor.connection.alias, model):
		if router.allow_migrate(schema_editor.connection.alias, model) and not model._meta.proxy:
		schema_editor.create_model(model)

		def references_model(self, name, app_label=None):

django/db/migrations/optimizer.py

+2 −1

Original line number	Diff line number	Diff line
		@@ -163,7 +163,8 @@ class MigrationOptimizer(object):
		"""
		Folds a CreateModel and a DeleteModel into nothing.
		"""
		if operation.name.lower() == other.name.lower():
		if operation.name.lower() == other.name.lower() and \
		not operation.options.get("proxy", False):
		return []

		def reduce_model_alter_delete(self, operation, other, in_between):

tests/migrations/test_autodetector.py

+50 −13

Original line number	Diff line number	Diff line
		@@ -37,7 +37,9 @@ class AutodetectorTests(TestCase):
		author_with_publisher = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("publisher", models.ForeignKey("testapp.Publisher"))])
		author_with_custom_user = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("user", models.ForeignKey("thirdapp.CustomUser"))])
		author_proxy = ModelState("testapp", "AuthorProxy", [], {"proxy": True}, ("testapp.author", ))
		author_proxy_options = ModelState("testapp", "AuthorProxy", [], {"proxy": True, "verbose_name": "Super Author"}, ("testapp.author", ))
		author_proxy_notproxy = ModelState("testapp", "AuthorProxy", [], {}, ("testapp.author", ))
		author_proxy_third = ModelState("thirdapp", "AuthorProxy", [], {"proxy": True}, ("testapp.author", ))
		author_unmanaged = ModelState("testapp", "AuthorUnmanaged", [], {"managed": False}, ("testapp.author", ))
		author_unmanaged_managed = ModelState("testapp", "AuthorUnmanaged", [], {}, ("testapp.author", ))
		author_with_m2m = ModelState("testapp", "Author", [
		@@ -54,6 +56,7 @@ class AutodetectorTests(TestCase):
		other_stable = ModelState("otherapp", "Stable", [("id", models.AutoField(primary_key=True))])
		third_thing = ModelState("thirdapp", "Thing", [("id", models.AutoField(primary_key=True))])
		book = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))])
		book_proxy_fk = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("thirdapp.AuthorProxy")), ("title", models.CharField(max_length=200))])
		book_with_no_author = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("title", models.CharField(max_length=200))])
		book_with_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))])
		book_with_field_and_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("writer", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))])
		@@ -363,6 +366,29 @@ class AutodetectorTests(TestCase):
		self.assertEqual(migration2.dependencies, [("testapp", "auto_1")])
		self.assertEqual(migration3.dependencies, [("otherapp", "auto_1")])

		def test_proxy_fk_dependency(self):
		"Tests that FK dependencies still work on proxy models"
		# Make state
		# Note that testapp (author) has no dependencies,
		# otherapp (book) depends on testapp (authorproxy)
		before = self.make_project_state([])
		after = self.make_project_state([self.author_empty, self.author_proxy_third, self.book_proxy_fk])
		autodetector = MigrationAutodetector(before, after)
		changes = autodetector._detect_changes()
		# Right number of migrations?
		self.assertNumberMigrations(changes, 'testapp', 1)
		self.assertNumberMigrations(changes, 'otherapp', 1)
		self.assertNumberMigrations(changes, 'thirdapp', 1)
		# Right number of actions?
		# Right actions?
		self.assertOperationTypes(changes, 'otherapp', 0, ["CreateModel"])
		self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel"])
		self.assertOperationTypes(changes, 'thirdapp', 0, ["CreateModel"])
		# Right dependencies?
		self.assertEqual(changes['testapp'][0].dependencies, [])
		self.assertEqual(changes['otherapp'][0].dependencies, [("thirdapp", "auto_1")])
		self.assertEqual(changes['thirdapp'][0].dependencies, [("testapp", "auto_1")])

		def test_same_app_no_fk_dependency(self):
		"""
		Tests that a migration with a FK between two models of the same app
		@@ -537,30 +563,29 @@ class AutodetectorTests(TestCase):
		self.assertEqual(action2.__class__.__name__, "AlterUniqueTogether")
		self.assertEqual(action2.unique_together, set([("title", "newfield")]))

		def test_proxy_ignorance(self):
		"Tests that the autodetector correctly ignores proxy models"
		def test_proxy(self):
		"Tests that the autodetector correctly deals with proxy models"
		# First, we test adding a proxy model
		before = self.make_project_state([self.author_empty])
		after = self.make_project_state([self.author_empty, self.author_proxy])
		autodetector = MigrationAutodetector(before, after)
		changes = autodetector._detect_changes()
		# Right number of migrations?
		self.assertEqual(len(changes), 0)
		self.assertNumberMigrations(changes, "testapp", 1)
		self.assertOperationTypes(changes, "testapp", 0, ["CreateModel"])
		self.assertOperationAttributes(changes, "testapp", 0, 0, name="AuthorProxy", options={"proxy": True})

		# Now, we test turning a proxy model into a non-proxy model
		# It should delete the proxy then make the real one
		before = self.make_project_state([self.author_empty, self.author_proxy])
		after = self.make_project_state([self.author_empty, self.author_proxy_notproxy])
		autodetector = MigrationAutodetector(before, after)
		changes = autodetector._detect_changes()
		# Right number of migrations?
		self.assertEqual(len(changes['testapp']), 1)
		# Right number of actions?
		migration = changes['testapp'][0]
		self.assertEqual(len(migration.operations), 1)
		# Right action?
		action = migration.operations[0]
		self.assertEqual(action.__class__.__name__, "CreateModel")
		self.assertEqual(action.name, "AuthorProxy")
		self.assertNumberMigrations(changes, "testapp", 1)
		self.assertOperationTypes(changes, "testapp", 0, ["DeleteModel", "CreateModel"])
		self.assertOperationAttributes(changes, "testapp", 0, 0, name="AuthorProxy")
		self.assertOperationAttributes(changes, "testapp", 0, 1, name="AuthorProxy", options={})

		def test_unmanaged_ignorance(self):
		"Tests that the autodetector correctly ignores managed models"
		@@ -804,8 +829,7 @@ class AutodetectorTests(TestCase):

		def test_alter_model_options(self):
		"""
		If two models with a ForeignKey from one to the other are removed at the same time,
		the autodetector should remove them in the correct order.
		Changing a model's options should make a change
		"""
		before = self.make_project_state([self.author_empty])
		after = self.make_project_state([self.author_with_options])
		@@ -816,6 +840,19 @@ class AutodetectorTests(TestCase):
		# Right actions in right order?
		self.assertOperationTypes(changes, "testapp", 0, ["AlterModelOptions"])

		def test_alter_model_options_proxy(self):
		"""
		Changing a proxy model's options should also make a change
		"""
		before = self.make_project_state([self.author_proxy, self.author_empty])
		after = self.make_project_state([self.author_proxy_options, self.author_empty])
		autodetector = MigrationAutodetector(before, after)
		changes = autodetector._detect_changes()
		# Right number of migrations?
		self.assertNumberMigrations(changes, "testapp", 1)
		# Right actions in right order?
		self.assertOperationTypes(changes, "testapp", 0, ["AlterModelOptions"])

		def test_set_alter_order_with_respect_to(self):
		"Tests that setting order_with_respect_to adds a field"
		# Make state

tests/migrations/test_operations.py

+60 −1

Original line number	Diff line number	Diff line
		@@ -37,7 +37,7 @@ class OperationTests(MigrationTestBase):
		with connection.schema_editor() as editor:
		return migration.unapply(project_state, editor)

		def set_up_test_model(self, app_label, second_model=False, third_model=False, related_model=False, mti_model=False):
		def set_up_test_model(self, app_label, second_model=False, third_model=False, related_model=False, mti_model=False, proxy_model=False):
		"""
		Creates a test model state and database table.
		"""
		@@ -112,6 +112,13 @@ class OperationTests(MigrationTestBase):
		],
		bases=['%s.Pony' % app_label],
		))
		if proxy_model:
		operations.append(migrations.CreateModel(
		"ProxyPony",
		fields=[],
		options={"proxy": True},
		bases=['%s.Pony' % app_label],
		))

		return self.apply_operations(app_label, ProjectState(), operations)

		@@ -221,6 +228,34 @@ class OperationTests(MigrationTestBase):
		operation.database_backwards("test_crmoih", editor, new_state, project_state)
		self.assertTableNotExists("test_crmoih_shetlandpony")

		def test_create_proxy_model(self):
		"""
		Tests that CreateModel ignores proxy models.
		"""
		project_state = self.set_up_test_model("test_crprmo")
		# Test the state alteration
		operation = migrations.CreateModel(
		"ProxyPony",
		[],
		options = {"proxy": True},
		bases = ("test_crprmo.Pony", ),
		)
		new_state = project_state.clone()
		operation.state_forwards("test_crprmo", new_state)
		self.assertIn(("test_crprmo", "proxypony"), new_state.models)
		# Test the database alteration
		self.assertTableNotExists("test_crprmo_proxypony")
		self.assertTableExists("test_crprmo_pony")
		with connection.schema_editor() as editor:
		operation.database_forwards("test_crprmo", editor, project_state, new_state)
		self.assertTableNotExists("test_crprmo_proxypony")
		self.assertTableExists("test_crprmo_pony")
		# And test reversal
		with connection.schema_editor() as editor:
		operation.database_backwards("test_crprmo", editor, new_state, project_state)
		self.assertTableNotExists("test_crprmo_proxypony")
		self.assertTableExists("test_crprmo_pony")

		def test_delete_model(self):
		"""
		Tests the DeleteModel operation.
		@@ -241,6 +276,30 @@ class OperationTests(MigrationTestBase):
		operation.database_backwards("test_dlmo", editor, new_state, project_state)
		self.assertTableExists("test_dlmo_pony")

		def test_delete_proxy_model(self):
		"""
		Tests the DeleteModel operation ignores proxy models.
		"""
		project_state = self.set_up_test_model("test_dlprmo", proxy_model=True)
		# Test the state alteration
		operation = migrations.DeleteModel("ProxyPony")
		new_state = project_state.clone()
		operation.state_forwards("test_dlprmo", new_state)
		self.assertIn(("test_dlprmo", "proxypony"), project_state.models)
		self.assertNotIn(("test_dlprmo", "proxypony"), new_state.models)
		# Test the database alteration
		self.assertTableExists("test_dlprmo_pony")
		self.assertTableNotExists("test_dlprmo_proxypony")
		with connection.schema_editor() as editor:
		operation.database_forwards("test_dlprmo", editor, project_state, new_state)
		self.assertTableExists("test_dlprmo_pony")
		self.assertTableNotExists("test_dlprmo_proxypony")
		# And test reversal
		with connection.schema_editor() as editor:
		operation.database_backwards("test_dlprmo", editor, new_state, project_state)
		self.assertTableExists("test_dlprmo_pony")
		self.assertTableNotExists("test_dlprmo_proxypony")

		def test_rename_model(self):
		"""
		Tests the RenameModel operation.