Fixed #22568: Better proxy model support in migrations

5 files changed, 220 insertions, 39 deletions

diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py
index e937e10b80..735b77441a 100644
--- a/django/db/migrations/autodetector.py
+++ b/django/db/migrations/autodetector.py
@@ -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,23 +105,32 @@ 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:
-                self.old_model_keys.append((al, mn))
-        self.new_model_keys = []
+            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))
+
         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)
             ):
-                self.new_model_keys.append((al, mn))
+                if model._meta.proxy:
+                    self.new_proxy_keys.append((al, mn))
+                else:
+                    self.new_model_keys.append((al, mn))
 
         # Renames have to come first
         self.generate_renamed_models()
@@ -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:
-                                    operation_dependencies.add((dep[0], "__latest__"))
+                                    # 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]
diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py
index d8f98b7926..a3c0cc74b2 100644
--- a/django/db/migrations/operations/models.py
+++ b/django/db/migrations/operations/models.py
@@ -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):
diff --git a/django/db/migrations/optimizer.py b/django/db/migrations/optimizer.py
index 2f6ab6be4a..342b88e99d 100644
--- a/django/db/migrations/optimizer.py
+++ b/django/db/migrations/optimizer.py
@@ -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):
diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py
index 9a8be73f27..b1233dc41f 100644
--- a/tests/migrations/test_autodetector.py
+++ b/tests/migrations/test_autodetector.py
@@ -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
diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py
index 64703d9140..9f5cad0391 100644
--- a/tests/migrations/test_operations.py
+++ b/tests/migrations/test_operations.py
@@ -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.