From cdeff3acc2b7d77b7f309db7e98878e08ad9ff25 Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Fri, 6 Sep 2013 12:14:09 -0500 Subject: [PATCH] Project/ModelState now correctly serialize multi-model inheritance --- django/db/migrations/state.py | 37 ++++++++++++++--- tests/migrations/test_state.py | 72 ++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 5 deletions(-) diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index f4e0325472a..0e532d3fdce 100644 --- a/django/db/migrations/state.py +++ b/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(): - model.render(self.app_cache) + # 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, ) diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 1a903cb4f26..2a6fef0ac19 100644 --- a/tests/migrations/test_state.py +++ b/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") @@ -54,6 +62,12 @@ class StateTests(TestCase): self.assertEqual(book_state.fields[2][1].null, False) 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): """ @@ -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)