Fixed #22239 -- Add auto detection of renamed models

This commit is contained in:
Chris Beaven 2014-03-10 13:38:24 +13:00
parent c564277937
commit 7feb70eef3
3 changed files with 140 additions and 8 deletions

View File

@ -61,6 +61,48 @@ class MigrationAutodetector(object):
for al, mn in self.to_state.models.keys() for al, mn in self.to_state.models.keys()
if not new_apps.get_model(al, mn)._meta.proxy 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. # Adding models. Phase 1 is adding models with no outward relationships.
added_models = set(new_model_keys) - set(old_model_keys) added_models = set(new_model_keys) - set(old_model_keys)
pending_add = {} pending_add = {}
@ -180,7 +222,8 @@ class MigrationAutodetector(object):
new_fields = set() new_fields = set()
unique_together_operations = [] unique_together_operations = []
for app_label, model_name in kept_models: 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] new_model_state = self.to_state.models[app_label, model_name]
# Collect field changes for later global dealing with (so AddFields # Collect field changes for later global dealing with (so AddFields
# always come before AlterFields even on separate models) # always come before AlterFields even on separate models)
@ -197,8 +240,10 @@ class MigrationAutodetector(object):
) )
)) ))
# New fields # New fields
renamed_fields = {}
for app_label, model_name, field_name in new_fields - old_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] new_model_state = self.to_state.models[app_label, model_name]
field = new_model_state.get_field_by_name(field_name) field = new_model_state.get_field_by_name(field_name)
# Scan to see if this is actually a rename! # Scan to see if this is actually a rename!
@ -206,7 +251,12 @@ class MigrationAutodetector(object):
found_rename = False found_rename = False
for rem_app_label, rem_model_name, rem_field_name in (old_fields - new_fields): 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 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): if self.questioner.ask_rename(model_name, rem_field_name, field_name, field):
self.add_to_migration( self.add_to_migration(
app_label, app_label,
@ -217,7 +267,8 @@ class MigrationAutodetector(object):
) )
) )
old_fields.remove((rem_app_label, rem_model_name, rem_field_name)) 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 found_rename = True
break break
if found_rename: if found_rename:
@ -250,7 +301,8 @@ class MigrationAutodetector(object):
self.add_swappable_dependency(app_label, swappable_setting) self.add_swappable_dependency(app_label, swappable_setting)
# Old fields # Old fields
for app_label, model_name, field_name in old_fields - new_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] new_model_state = self.to_state.models[app_label, model_name]
self.add_to_migration( self.add_to_migration(
app_label, app_label,
@ -262,10 +314,12 @@ class MigrationAutodetector(object):
# The same fields # The same fields
for app_label, model_name, field_name in old_fields.intersection(new_fields): for app_label, model_name, field_name in old_fields.intersection(new_fields):
# Did the field change? # 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] new_model_state = self.to_state.models[app_label, model_name]
old_field_dec = old_model_state.get_field_by_name(field_name).deconstruct() old_field_name = renamed_fields.get((app_label, model_name, field_name), field_name)
new_field_dec = new_model_state.get_field_by_name(field_name).deconstruct() 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: if old_field_dec != new_field_dec:
self.add_to_migration( self.add_to_migration(
app_label, app_label,

View File

@ -56,6 +56,10 @@ class MigrationQuestioner(object):
"Was this field really renamed?" "Was this field really renamed?"
return self.defaults.get("ask_rename", False) 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): def ask_merge(self, app_label):
"Do you really want to merge these migrations?" "Do you really want to merge these migrations?"
return self.defaults.get("ask_merge", False) return self.defaults.get("ask_merge", False)
@ -123,6 +127,10 @@ class InteractiveMigrationQuestioner(MigrationQuestioner):
"Was this field really renamed?" "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) 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): def ask_merge(self, app_label):
return self._boolean_input( return self._boolean_input(
"\nMerging will only work if the operations printed above do not conflict\n" + "\nMerging will only work if the operations printed above do not conflict\n" +

View File

@ -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_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_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_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_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_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", )) 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))]) other_stable = ModelState("otherapp", "Stable", [("id", models.AutoField(primary_key=True))])
third_thing = ModelState("thirdapp", "Thing", [("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 = 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 = 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_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")]}) 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.old_name, "name")
self.assertEqual(action.new_name, "names") 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): def test_fk_dependency(self):
"Tests that having a ForeignKey automatically adds a dependency" "Tests that having a ForeignKey automatically adds a dependency"
# Make state # Make state