Project/ModelState now correctly serialize multi-model inheritance

django/db/migrations/state.py

@@ -1,9 +1,14 @@
 from django.db import models
 from django.db.models.loading import BaseAppCache
 from django.db.models.options import DEFAULT_NAMES
+from django.utils import six
 from django.utils.module_loading import import_by_path
 
 
+class InvalidBasesError(ValueError):
+    pass
+
+
 class ProjectState(object):
     """
     Represents the entire project's overall state.
@@ -28,8 +33,21 @@ class ProjectState(object):
         "Turns the project state into actual models in a new AppCache"
         if self.app_cache is None:
             self.app_cache = BaseAppCache()
-            for model in self.models.values():
+            # We keep trying to render the models in a loop, ignoring invalid
+            # base errors, until the size of the unrendered models doesn't
+            # decrease by at least one, meaning there's a base dependency loop/
+            # missing base.
+            unrendered_models = list(self.models.values())
+            while unrendered_models:
+                new_unrendered_models = []
+                for model in unrendered_models:
+                    try:
                         model.render(self.app_cache)
+                    except InvalidBasesError:
+                        new_unrendered_models.append(model)
+                if len(new_unrendered_models) == len(unrendered_models):
+                    raise InvalidBasesError("Cannot resolve bases for %r" % new_unrendered_models)
+                unrendered_models = new_unrendered_models
         return self.app_cache
 
     @classmethod
@@ -86,7 +104,11 @@ class ModelState(object):
                 else:
                     options[name] = model._meta.original_attrs[name]
         # Make our record
-        bases = tuple(model for model in model.__bases__ if (not hasattr(model, "_meta") or not model._meta.abstract))
+        bases = tuple(
+            ("%s.%s" % (base._meta.app_label, base._meta.object_name.lower()) if hasattr(base, "_meta") else base)
+            for base in model.__bases__
+            if (not hasattr(base, "_meta") or not base._meta.abstract)
+        )
         if not bases:
             bases = (models.Model, )
         return cls(
@@ -123,7 +145,12 @@ class ModelState(object):
             meta_contents["unique_together"] = list(meta_contents["unique_together"])
         meta = type("Meta", tuple(), meta_contents)
         # Then, work out our bases
-        # TODO: Use the actual bases
+        bases = tuple(
+            (app_cache.get_model(*base.split(".", 1)) if isinstance(base, six.string_types) else base)
+            for base in self.bases
+        )
+        if None in bases:
+            raise InvalidBasesError("Cannot resolve one or more bases from %r" % self.bases)
         # Turn fields into a dict for the body, add other bits
         body = dict(self.fields)
         body['Meta'] = meta
@@ -131,7 +158,7 @@ class ModelState(object):
         # Then, make a Model object
         return type(
             self.name,
-            tuple(self.bases),
+            bases,
             body,
         )
 

tests/migrations/test_state.py

@@ -25,6 +25,13 @@ class StateTests(TestCase):
                 app_cache = new_app_cache
                 unique_together = ["name", "bio"]
 
+        class AuthorProxy(Author):
+            class Meta:
+                app_label = "migrations"
+                app_cache = new_app_cache
+                proxy = True
+                ordering = ["name"]
+
         class Book(models.Model):
             title = models.CharField(max_length=1000)
             author = models.ForeignKey(Author)
@@ -36,6 +43,7 @@ class StateTests(TestCase):
 
         project_state = ProjectState.from_app_cache(new_app_cache)
         author_state = project_state.models['migrations', 'author']
+        author_proxy_state = project_state.models['migrations', 'authorproxy']
         book_state = project_state.models['migrations', 'book']
         
         self.assertEqual(author_state.app_label, "migrations")
@@ -55,6 +63,12 @@ class StateTests(TestCase):
         self.assertEqual(book_state.options, {"verbose_name": "tome", "db_table": "test_tome"})
         self.assertEqual(book_state.bases, (models.Model, ))
         
+        self.assertEqual(author_proxy_state.app_label, "migrations")
+        self.assertEqual(author_proxy_state.name, "AuthorProxy")
+        self.assertEqual(author_proxy_state.fields, [])
+        self.assertEqual(author_proxy_state.options, {"proxy": True, "ordering": ["name"]})
+        self.assertEqual(author_proxy_state.bases, ("migrations.author", ))
+
     def test_render(self):
         """
         Tests rendering a ProjectState into an AppCache.
@@ -92,5 +106,63 @@ class StateTests(TestCase):
                 app_label = "migrations"
                 app_cache = new_app_cache
 
+        # First, test rendering individually
         yet_another_app_cache = BaseAppCache()
+
+        # We shouldn't be able to render yet
+        with self.assertRaises(ValueError):
             ModelState.from_model(Novel).render(yet_another_app_cache)
+
+        # Once the parent model is in the app cache, it should be fine
+        ModelState.from_model(Book).render(yet_another_app_cache)
+        ModelState.from_model(Novel).render(yet_another_app_cache)
+
+    def test_render_project_dependencies(self):
+        """
+        Tests that the ProjectState render method correctly renders models
+        to account for inter-model base dependencies.
+        """
+        new_app_cache = BaseAppCache()
+
+        class A(models.Model):
+            class Meta:
+                app_label = "migrations"
+                app_cache = new_app_cache
+
+        class B(A):
+            class Meta:
+                app_label = "migrations"
+                app_cache = new_app_cache
+
+        class C(B):
+            class Meta:
+                app_label = "migrations"
+                app_cache = new_app_cache
+
+        class D(A):
+            class Meta:
+                app_label = "migrations"
+                app_cache = new_app_cache
+
+        class E(B):
+            class Meta:
+                app_label = "migrations"
+                app_cache = new_app_cache
+                proxy = True
+
+        class F(D):
+            class Meta:
+                app_label = "migrations"
+                app_cache = new_app_cache
+                proxy = True
+
+        # Make a ProjectState and render it
+        project_state = ProjectState()
+        project_state.add_model_state(ModelState.from_model(A))
+        project_state.add_model_state(ModelState.from_model(B))
+        project_state.add_model_state(ModelState.from_model(C))
+        project_state.add_model_state(ModelState.from_model(D))
+        project_state.add_model_state(ModelState.from_model(E))
+        project_state.add_model_state(ModelState.from_model(F))
+        final_app_cache = project_state.render()
+        self.assertEqual(len(final_app_cache.get_models()), 6)