From 25c157e4ccc27702883b0c3a53e0e9b44e19757d Mon Sep 17 00:00:00 2001 From: Alex Hill Date: Thu, 26 Mar 2015 10:09:58 +0800 Subject: [PATCH] Refs #24215 -- Improved error message for unhandled lazy model operations. --- django/db/migrations/state.py | 39 +++++++++++++++++++++++++++++++--- tests/migrations/test_state.py | 38 +++++++++++++++++++++++++++++---- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index 560d0014c8..3613ae2a4d 100644 --- a/django/db/migrations/state.py +++ b/django/db/migrations/state.py @@ -232,9 +232,42 @@ class StateApps(Apps): if ignore_swappable: pending_models -= {make_model_tuple(settings.AUTH_USER_MODEL)} if pending_models: - msg = "Unhandled pending operations for models: %s" - labels = (".".join(model_key) for model_key in self._pending_operations) - raise ValueError(msg % ", ".join(labels)) + raise ValueError(self._pending_models_error(pending_models)) + + def _pending_models_error(self, pending_models): + """ + Almost all internal uses of lazy operations are to resolve string model + references in related fields. We can extract the fields from those + operations and use them to provide a nicer error message. + + This will work for any function passed to lazy_related_operation() that + has a keyword argument called 'field'. + """ + def extract_field(operation): + # Expect a functools.partial() with a kwarg called 'field' applied. + try: + return operation.func.keywords['field'] + except (AttributeError, KeyError): + return None + + def extract_field_names(operations): + return (str(field) for field in map(extract_field, operations) if field) + + get_ops = self._pending_operations.__getitem__ + # Ordered list of pairs of the form + # ((app_label, model_name), [field_name_1, field_name_2, ...]) + models_fields = sorted( + (model_key, sorted(extract_field_names(get_ops(model_key)))) + for model_key in pending_models + ) + + def model_text(model_key, fields): + field_list = ", ".join(fields) + field_text = " (referred to by fields: %s)" % field_list if fields else "" + return ("%s.%s" % model_key) + field_text + + msg = "Unhandled pending operations for models:" + return "\n ".join([msg] + [model_text(*i) for i in models_fields]) @contextmanager def bulk_update(self): diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 3d059b32cb..d528e89920 100644 --- a/tests/migrations/test_state.py +++ b/tests/migrations/test_state.py @@ -625,8 +625,16 @@ class StateTests(SimpleTestCase): app_label = "migrations" apps = new_apps + class Publisher(models.Model): + name = models.TextField() + + class Meta: + app_label = "migrations" + apps = new_apps + class Book(models.Model): author = models.ForeignKey(Author, models.CASCADE) + publisher = models.ForeignKey(Publisher, models.CASCADE) class Meta: app_label = "migrations" @@ -642,20 +650,42 @@ class StateTests(SimpleTestCase): # Make a valid ProjectState and render it project_state = ProjectState() project_state.add_model(ModelState.from_model(Author)) + project_state.add_model(ModelState.from_model(Publisher)) project_state.add_model(ModelState.from_model(Book)) project_state.add_model(ModelState.from_model(Magazine)) - self.assertEqual(len(project_state.apps.get_models()), 3) + self.assertEqual(len(project_state.apps.get_models()), 4) # now make an invalid one with a ForeignKey project_state = ProjectState() project_state.add_model(ModelState.from_model(Book)) - with self.assertRaises(ValueError): + msg = ( + "Unhandled pending operations for models:\n" + " migrations.author (referred to by fields: migrations.Book.author)\n" + " migrations.publisher (referred to by fields: migrations.Book.publisher)" + ) + with self.assertRaisesMessage(ValueError, msg): project_state.apps - # and another with ManyToManyField + # And another with ManyToManyField. project_state = ProjectState() project_state.add_model(ModelState.from_model(Magazine)) - with self.assertRaises(ValueError): + msg = ( + "Unhandled pending operations for models:\n" + " migrations.author (referred to by fields: " + "migrations.Magazine.authors, migrations.Magazine_authors.author)" + ) + with self.assertRaisesMessage(ValueError, msg): + project_state.apps + + # And now with multiple models and multiple fields. + project_state.add_model(ModelState.from_model(Book)) + msg = ( + "Unhandled pending operations for models:\n" + " migrations.author (referred to by fields: migrations.Book.author, " + "migrations.Magazine.authors, migrations.Magazine_authors.author)\n" + " migrations.publisher (referred to by fields: migrations.Book.publisher)" + ) + with self.assertRaisesMessage(ValueError, msg): project_state.apps def test_real_apps(self):