From 7feb70eef3e8e76315e2535ccec94ac4ce34d7d2 Mon Sep 17 00:00:00 2001 From: Chris Beaven Date: Mon, 10 Mar 2014 13:38:24 +1300 Subject: [PATCH] Fixed #22239 -- Add auto detection of renamed models --- django/db/migrations/autodetector.py | 70 ++++++++++++++++++++++++--- django/db/migrations/questioner.py | 8 +++ tests/migrations/test_autodetector.py | 70 +++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 8 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 2ea96cac0b..a9f6b610de 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -61,6 +61,48 @@ class MigrationAutodetector(object): for al, mn in self.to_state.models.keys() if not new_apps.get_model(al, mn)._meta.proxy ] + + def _rel_agnostic_fields_def(fields): + """ + Return a definition of the fields that ignores field names and + what related fields actually relate to. + """ + fields_def = [] + for name, field in fields: + deconstruction = field.deconstruct()[1:] + if field.rel and field.rel.to: + del deconstruction[2]['to'] + fields_def.append(deconstruction) + return fields_def + + # Find any renamed models. + renamed_models = {} + renamed_models_rel = {} + added_models = set(new_model_keys) - set(old_model_keys) + for app_label, model_name in added_models: + model_state = self.to_state.models[app_label, model_name] + model_fields_def = _rel_agnostic_fields_def(model_state.fields) + + removed_models = set(old_model_keys) - set(new_model_keys) + for rem_app_label, rem_model_name in removed_models: + if rem_app_label == app_label: + rem_model_state = self.from_state.models[rem_app_label, rem_model_name] + rem_model_fields_def = _rel_agnostic_fields_def(rem_model_state.fields) + if model_fields_def == rem_model_fields_def: + if self.questioner.ask_rename_model(rem_model_state, model_state): + self.add_to_migration( + app_label, + operations.RenameModel( + old_name=rem_model_name, + new_name=model_name, + ) + ) + renamed_models[app_label, model_name] = rem_model_name + renamed_models_rel['%s.%s' % (rem_model_state.app_label, rem_model_state.name)] = '%s.%s' % (model_state.app_label, model_state.name) + old_model_keys.remove((rem_app_label, rem_model_name)) + old_model_keys.append((app_label, model_name)) + break + # Adding models. Phase 1 is adding models with no outward relationships. added_models = set(new_model_keys) - set(old_model_keys) pending_add = {} @@ -180,7 +222,8 @@ class MigrationAutodetector(object): new_fields = set() unique_together_operations = [] for app_label, model_name in kept_models: - old_model_state = self.from_state.models[app_label, model_name] + old_model_name = 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] # Collect field changes for later global dealing with (so AddFields # always come before AlterFields even on separate models) @@ -197,8 +240,10 @@ class MigrationAutodetector(object): ) )) # New fields + renamed_fields = {} for app_label, model_name, field_name in new_fields - old_fields: - old_model_state = self.from_state.models[app_label, model_name] + old_model_name = 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] field = new_model_state.get_field_by_name(field_name) # Scan to see if this is actually a rename! @@ -206,7 +251,12 @@ class MigrationAutodetector(object): found_rename = False for rem_app_label, rem_model_name, rem_field_name in (old_fields - new_fields): if rem_app_label == app_label and rem_model_name == model_name: - if old_model_state.get_field_by_name(rem_field_name).deconstruct()[1:] == field_dec: + old_field_dec = old_model_state.get_field_by_name(rem_field_name).deconstruct()[1:] + if field.rel and field.rel.to: + old_rel_to = old_field_dec[2]['to'] + if old_rel_to in renamed_models_rel: + old_field_dec[2]['to'] = renamed_models_rel[old_rel_to] + if old_field_dec == field_dec: if self.questioner.ask_rename(model_name, rem_field_name, field_name, field): self.add_to_migration( app_label, @@ -217,7 +267,8 @@ class MigrationAutodetector(object): ) ) old_fields.remove((rem_app_label, rem_model_name, rem_field_name)) - new_fields.remove((app_label, model_name, field_name)) + old_fields.add((app_label, model_name, field_name)) + renamed_fields[app_label, model_name, field_name] = rem_field_name found_rename = True break if found_rename: @@ -250,7 +301,8 @@ class MigrationAutodetector(object): self.add_swappable_dependency(app_label, swappable_setting) # Old fields for app_label, model_name, field_name in old_fields - new_fields: - old_model_state = self.from_state.models[app_label, model_name] + old_model_name = 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] self.add_to_migration( app_label, @@ -262,10 +314,12 @@ class MigrationAutodetector(object): # The same fields for app_label, model_name, field_name in old_fields.intersection(new_fields): # Did the field change? - old_model_state = self.from_state.models[app_label, model_name] + old_model_name = 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] - old_field_dec = old_model_state.get_field_by_name(field_name).deconstruct() - new_field_dec = new_model_state.get_field_by_name(field_name).deconstruct() + old_field_name = renamed_fields.get((app_label, model_name, field_name), field_name) + old_field_dec = old_model_state.get_field_by_name(old_field_name).deconstruct()[1:] + new_field_dec = new_model_state.get_field_by_name(field_name).deconstruct()[1:] if old_field_dec != new_field_dec: self.add_to_migration( app_label, diff --git a/django/db/migrations/questioner.py b/django/db/migrations/questioner.py index f214d243b2..3f8cefeecd 100644 --- a/django/db/migrations/questioner.py +++ b/django/db/migrations/questioner.py @@ -56,6 +56,10 @@ class MigrationQuestioner(object): "Was this field really renamed?" return self.defaults.get("ask_rename", False) + def ask_rename_model(self, old_model_state, new_model_state): + "Was this model really renamed?" + return self.defaults.get("ask_rename_model", False) + def ask_merge(self, app_label): "Do you really want to merge these migrations?" return self.defaults.get("ask_merge", False) @@ -123,6 +127,10 @@ class InteractiveMigrationQuestioner(MigrationQuestioner): "Was this field really renamed?" return self._boolean_input("Did you rename %s.%s to %s.%s (a %s)? [y/N]" % (model_name, old_name, model_name, new_name, field_instance.__class__.__name__), False) + def ask_rename_model(self, old_model_state, new_model_state): + "Was this model really renamed?" + return self._boolean_input("Did you rename the %s.%s model to %s? [y/N]" % (old_model_state.app_label, old_model_state.name, new_model_state.name), False) + def ask_merge(self, app_label): return self._boolean_input( "\nMerging will only work if the operations printed above do not conflict\n" + diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index c6133e6227..eefee6fa76 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -18,6 +18,7 @@ class AutodetectorTests(TestCase): author_name_renamed = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("names", models.CharField(max_length=200))]) author_name_default = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default='Ada Lovelace'))]) author_with_book = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("book", models.ForeignKey("otherapp.Book"))]) + author_renamed_with_book = ModelState("testapp", "Writer", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("book", models.ForeignKey("otherapp.Book"))]) 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", )) @@ -29,6 +30,8 @@ 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_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))]) book_unique = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("author", "title")]}) book_unique_2 = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("title", "author")]}) book_unique_3 = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("newfield", models.IntegerField()), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("title", "newfield")]}) @@ -184,6 +187,73 @@ class AutodetectorTests(TestCase): self.assertEqual(action.old_name, "name") self.assertEqual(action.new_name, "names") + def test_rename_model(self): + "Tests autodetection of renamed models" + # Make state + before = self.make_project_state([self.author_with_book, self.book]) + after = self.make_project_state([self.author_renamed_with_book, self.book_with_author_renamed]) + autodetector = MigrationAutodetector(before, after, MigrationQuestioner({"ask_rename_model": True})) + changes = autodetector._detect_changes() + + # Right number of migrations for model rename? + 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__, "RenameModel") + self.assertEqual(action.old_name, "author") + self.assertEqual(action.new_name, "writer") + + # Right number of migrations for related field rename? + self.assertEqual(len(changes['otherapp']), 1) + # Right number of actions? + migration = changes['otherapp'][0] + self.assertEqual(len(migration.operations), 1) + # Right action? + action = migration.operations[0] + self.assertEqual(action.__class__.__name__, "AlterField") + self.assertEqual(action.name, "author") + self.assertEqual(action.field.rel.to.__name__, "Writer") + + def test_rename_model_with_renamed_rel_field(self): + """ + Tests autodetection of renamed models while simultaneously renaming one + of the fields that relate to the renamed model. + """ + # Make state + before = self.make_project_state([self.author_with_book, self.book]) + after = self.make_project_state([self.author_renamed_with_book, self.book_with_field_and_author_renamed]) + autodetector = MigrationAutodetector(before, after, MigrationQuestioner({"ask_rename_model": True, "ask_rename": True})) + changes = autodetector._detect_changes() + + # Right number of migrations for model rename? + self.assertEqual(len(changes['testapp']), 1) + # Right number of actions? + migration = changes['testapp'][0] + self.assertEqual(len(migration.operations), 1) + # Right actions? + action = migration.operations[0] + self.assertEqual(action.__class__.__name__, "RenameModel") + self.assertEqual(action.old_name, "author") + self.assertEqual(action.new_name, "writer") + + # Right number of migrations for related field rename? + self.assertEqual(len(changes['otherapp']), 1) + # Right number of actions? + migration = changes['otherapp'][0] + self.assertEqual(len(migration.operations), 2) + # Right actions? + action = migration.operations[0] + self.assertEqual(action.__class__.__name__, "RenameField") + self.assertEqual(action.old_name, "author") + self.assertEqual(action.new_name, "writer") + action = migration.operations[1] + self.assertEqual(action.__class__.__name__, "AlterField") + self.assertEqual(action.name, "writer") + self.assertEqual(action.field.rel.to.__name__, "Writer") + def test_fk_dependency(self): "Tests that having a ForeignKey automatically adds a dependency" # Make state