Merge pull request #2396 from loic/ticket21893

Fixed #21893 -- ModelState didn't account for MTI parents inherited from abstract models.

django/db/migrations/state.py

@@ -151,6 +151,23 @@ class ModelState(object):
                     options[name] = set(normalize_together(it))
                 else:
                     options[name] = model._meta.original_attrs[name]
+
+        def flatten_bases(model):
+            bases = []
+            for base in model.__bases__:
+                if hasattr(base, "_meta") and base._meta.abstract:
+                    bases.extend(flatten_bases(base))
+                else:
+                    bases.append(base)
+            return bases
+
+        # We can't rely on __mro__ directly because we only want to flatten
+        # abstract models and not the whole tree. However by recursing on
+        # __bases__ we may end up with duplicates and ordering issues, we
+        # therefore discard any duplicates and reorder the bases according
+        # to their index in the MRO.
+        flattened_bases = sorted(set(flatten_bases(model)), key=lambda x:model.__mro__.index(x))
+
         # Make our record
         bases = tuple(
             (
@@ -158,8 +175,7 @@ class ModelState(object):
                 if hasattr(base, "_meta") else
                 base
             )
-            for base in model.__bases__
+            for base in flattened_bases
-            if (not hasattr(base, "_meta") or not base._meta.abstract)
         )
         # Ensure at least one base inherits from models.Model
         if not any((isinstance(base, six.string_types) or issubclass(base, models.Model)) for base in bases):

tests/migrations/test_operations.py

@@ -14,7 +14,7 @@ class OperationTests(MigrationTestBase):
     both forwards and backwards.
     """
 
-    def set_up_test_model(self, app_label, second_model=False, related_model=False):
+    def set_up_test_model(self, app_label, second_model=False, related_model=False, mti_model=False):
         """
         Creates a test model state and database table.
         """
@@ -38,7 +38,12 @@ class OperationTests(MigrationTestBase):
             ],
         )]
         if second_model:
-            operations.append(migrations.CreateModel("Stable", [("id", models.AutoField(primary_key=True))]))
+            operations.append(migrations.CreateModel(
+                "Stable",
+                [
+                    ("id", models.AutoField(primary_key=True)),
+                ]
+            ))
         if related_model:
             operations.append(migrations.CreateModel(
                 "Rider",
@@ -47,6 +52,21 @@ class OperationTests(MigrationTestBase):
                     ("pony", models.ForeignKey("Pony")),
                 ],
             ))
+        if mti_model:
+            operations.append(migrations.CreateModel(
+                "ShetlandPony",
+                fields=[
+                    ('pony_ptr', models.OneToOneField(
+                        auto_created=True,
+                        primary_key=True,
+                        to_field='id',
+                        serialize=False,
+                        to='Pony',
+                    )),
+                    ("cuteness", models.IntegerField(default=1)),
+                ],
+                bases=['%s.Pony' % app_label],
+            ))
         project_state = ProjectState()
         for operation in operations:
             operation.state_forwards(app_label, project_state)
@@ -495,7 +515,7 @@ class OperationTests(MigrationTestBase):
         Tests the RunPython operation
         """
 
-        project_state = self.set_up_test_model("test_runpython")
+        project_state = self.set_up_test_model("test_runpython", mti_model=True)
 
         # Create the operation
         def inner_method(models, schema_editor):
@@ -533,7 +553,34 @@ class OperationTests(MigrationTestBase):
             no_reverse_operation.database_forwards("test_runpython", editor, project_state, new_state)
             with self.assertRaises(NotImplementedError):
                 no_reverse_operation.database_backwards("test_runpython", editor, new_state, project_state)
+        self.assertEqual(project_state.render().get_model("test_runpython", "Pony").objects.count(), 2)
 
+        def create_ponies(models, schema_editor):
+            Pony = models.get_model("test_runpython", "Pony")
+            pony1 = Pony.objects.create(pink=1, weight=3.55)
+            self.assertIsNot(pony1.pk, None)
+            pony2 = Pony.objects.create(weight=5)
+            self.assertIsNot(pony2.pk, None)
+            self.assertNotEqual(pony1.pk, pony2.pk)
+
+        operation = migrations.RunPython(create_ponies)
+        with connection.schema_editor() as editor:
+            operation.database_forwards("test_runpython", editor, project_state, new_state)
+        self.assertEqual(project_state.render().get_model("test_runpython", "Pony").objects.count(), 4)
+
+        def create_shetlandponies(models, schema_editor):
+            ShetlandPony = models.get_model("test_runpython", "ShetlandPony")
+            pony1 = ShetlandPony.objects.create(weight=4.0)
+            self.assertIsNot(pony1.pk, None)
+            pony2 = ShetlandPony.objects.create(weight=5.0)
+            self.assertIsNot(pony2.pk, None)
+            self.assertNotEqual(pony1.pk, pony2.pk)
+
+        operation = migrations.RunPython(create_shetlandponies)
+        with connection.schema_editor() as editor:
+            operation.database_forwards("test_runpython", editor, project_state, new_state)
+        self.assertEqual(project_state.render().get_model("test_runpython", "Pony").objects.count(), 6)
+        self.assertEqual(project_state.render().get_model("test_runpython", "ShetlandPony").objects.count(), 2)
 
 class MigrateNothingRouter(object):
     """

tests/migrations/test_state.py

@@ -166,6 +166,16 @@ class StateTests(TestCase):
                 app_label = "migrations"
                 apps = Apps()
 
+        class AbstractSubFooBar(FooBar):
+            class Meta:
+                abstract = True
+                apps = Apps()
+
+        class SubFooBar(AbstractSubFooBar):
+            class Meta:
+                app_label = "migrations"
+                apps = Apps()
+
         apps = Apps(["migrations"])
 
         # We shouldn't be able to render yet
@@ -175,8 +185,13 @@ class StateTests(TestCase):
 
         # Once the parent models are in the app registry, it should be fine
         ModelState.from_model(Foo).render(apps)
+        self.assertSequenceEqual(ModelState.from_model(Foo).bases, [models.Model])
         ModelState.from_model(Bar).render(apps)
+        self.assertSequenceEqual(ModelState.from_model(Bar).bases, [models.Model])
         ModelState.from_model(FooBar).render(apps)
+        self.assertSequenceEqual(ModelState.from_model(FooBar).bases, ['migrations.foo', 'migrations.bar'])
+        ModelState.from_model(SubFooBar).render(apps)
+        self.assertSequenceEqual(ModelState.from_model(SubFooBar).bases, ['migrations.foobar'])
 
     def test_render_project_dependencies(self):
         """