Skip to content

Commit

Permalink
Fixed #31503 -- Moved unique/index_together removal in autodetector b…
Browse files Browse the repository at this point in the history 
…efore field alterations
  • Loading branch information
@David-Wobrock
David-Wobrock committed Oct 16, 2021
1 parent 569a335 commit cb37a15
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 17 deletions.
45 changes: 38 additions & 7 deletions django/db/migrations/autodetector.py
View file
Expand Up @@ -178,8 +178,12 @@ def _detect_changes(self, convert_apps=None, graph=None):
# Generate index removal operations before field is removed
self.generate_removed_constraints()
self.generate_removed_indexes()
# Generate field operations
# Generate field renaming operations
self.generate_renamed_fields()
# Generate removal of foo together
self.generate_removed_altered_unique_together()
self.generate_removed_altered_index_together()
# Generate field operations
self.generate_removed_fields()
self.generate_added_fields()
self.generate_altered_fields()
Expand Down Expand Up @@ -1111,8 +1115,7 @@ def _get_dependencies_for_foreign_key(app_label, model_name, field, project_stat
dependencies.append((through_app_label, through_object_name, None, True))
return dependencies

def _generate_altered_foo_together(self, operation):
option_name = operation.option_name
def _get_altered_foo_together_operations(self, option_name):
for app_label, model_name in sorted(self.kept_model_keys):
old_model_name = self.renamed_models.get((app_label, model_name), model_name)
old_model_state = self.from_state.models[app_label, old_model_name]
Expand Down Expand Up @@ -1140,13 +1143,41 @@ def _generate_altered_foo_together(self, operation):
dependencies.extend(self._get_dependencies_for_foreign_key(
app_label, model_name, field, self.to_state,
))
yield (
old_value,
new_value,
app_label,
model_name,
dependencies,
)

def _generate_removed_altered_foo_together(self, operation):
for old_value, new_value, app_label, model_name, dependencies in self._get_altered_foo_together_operations(
operation.option_name
):
removal_value = new_value.intersection(old_value)
if removal_value or old_value:
self.add_operation(
app_label,
operation(
name=model_name,
**{option_name: new_value}
),
operation(name=model_name, **{operation.option_name: removal_value}),
dependencies=dependencies,
)

def generate_removed_altered_unique_together(self):
self._generate_removed_altered_foo_together(operations.AlterUniqueTogether)

def generate_removed_altered_index_together(self):
self._generate_removed_altered_foo_together(operations.AlterIndexTogether)

def _generate_altered_foo_together(self, operation):
for old_value, new_value, app_label, model_name, dependencies in self._get_altered_foo_together_operations(
operation.option_name
):
removal_value = new_value.intersection(old_value)
if new_value != removal_value:
self.add_operation(
app_label,
operation(name=model_name, **{operation.option_name: new_value}),
dependencies=dependencies,
)

Expand Down
77 changes: 67 additions & 10 deletions tests/migrations/test_autodetector.py
View file
Expand Up @@ -1564,9 +1564,16 @@ def test_foo_together_ordering(self):
)
# Right number/type of migrations?
self.assertNumberMigrations(changes, "otherapp", 1)
self.assertOperationTypes(changes, "otherapp", 0, ["AlterUniqueTogether", "AlterIndexTogether"])
self.assertOperationAttributes(changes, "otherapp", 0, 0, name="book", unique_together={("title", "author")})
self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", index_together={("title", "author")})
self.assertOperationTypes(changes, "otherapp", 0, [
"AlterUniqueTogether",
"AlterIndexTogether",
"AlterUniqueTogether",
"AlterIndexTogether",
])
self.assertOperationAttributes(changes, "otherapp", 0, 0, name="book", unique_together=set())
self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", index_together=set())
self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", unique_together={("title", "author")})
self.assertOperationAttributes(changes, "otherapp", 0, 3, name="book", index_together={("title", "author")})

def test_add_field_and_foo_together(self):
"""
Expand Down Expand Up @@ -1613,10 +1620,52 @@ def test_remove_field_and_foo_together(self):
)
# Right number/type of migrations?
self.assertNumberMigrations(changes, "otherapp", 1)
self.assertOperationTypes(changes, "otherapp", 0, ["AlterUniqueTogether", "AlterIndexTogether", "RemoveField"])
self.assertOperationAttributes(changes, "otherapp", 0, 0, name="book", unique_together={("author", "title")})
self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", index_together={("author", "title")})
self.assertOperationAttributes(changes, "otherapp", 0, 2, model_name="book", name="newfield")
self.assertOperationTypes(changes, "otherapp", 0, [
"AlterUniqueTogether",
"AlterIndexTogether",
"AlterUniqueTogether",
"AlterIndexTogether",
"RemoveField",
])
self.assertOperationAttributes(changes, "otherapp", 0, 0, name="book", unique_together=set())
self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", index_together=set())
self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", unique_together={("author", "title")})
self.assertOperationAttributes(changes, "otherapp", 0, 3, name="book", index_together={("author", "title")})
self.assertOperationAttributes(changes, "otherapp", 0, 4, model_name="book", name="newfield")

def test_alter_field_and_foo_together(self):
"""
Fields will be altered after deleting some index/unique_together.
"""
initial_author = ModelState("testapp", "Author", [
("id", models.AutoField(primary_key=True)),
("name", models.CharField(max_length=200)),
("age", models.IntegerField(db_index=True)),
], {
"unique_together": {("name",)},
})
author_reversed_constraints = ModelState("testapp", "Author", [
("id", models.AutoField(primary_key=True)),
("name", models.CharField(max_length=200, unique=True)),
("age", models.IntegerField()),
], {
"index_together": {("age",)},
})
changes = self.get_changes(
[initial_author], [author_reversed_constraints]
)
# Right number/type of migrations?
self.assertNumberMigrations(changes, "testapp", 1)
self.assertOperationTypes(changes, "testapp", 0, [
"AlterUniqueTogether",
"AlterField",
"AlterField",
"AlterIndexTogether",
])
self.assertOperationAttributes(changes, "testapp", 0, 0, name="author", unique_together=set())
self.assertOperationAttributes(changes, "testapp", 0, 1, model_name="author", name="age")
self.assertOperationAttributes(changes, "testapp", 0, 2, model_name="author", name="name")
self.assertOperationAttributes(changes, "testapp", 0, 3, name="author", index_together={("age",)})

def test_rename_field_and_foo_together(self):
"""
Expand All @@ -1629,11 +1678,19 @@ def test_rename_field_and_foo_together(self):
)
# Right number/type of migrations?
self.assertNumberMigrations(changes, "otherapp", 1)
self.assertOperationTypes(changes, "otherapp", 0, ["RenameField", "AlterUniqueTogether", "AlterIndexTogether"])
self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", unique_together={
self.assertOperationTypes(changes, "otherapp", 0, [
"RenameField",
"AlterUniqueTogether",
"AlterIndexTogether",
"AlterUniqueTogether",
"AlterIndexTogether",
])
self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", unique_together=set())
self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", index_together=set())
self.assertOperationAttributes(changes, "otherapp", 0, 3, name="book", unique_together={
("title", "newfield2")
})
self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", index_together={("title", "newfield2")})
self.assertOperationAttributes(changes, "otherapp", 0, 4, name="book", index_together={("title", "newfield2")})

def test_proxy(self):
"""The autodetector correctly deals with proxy models."""
Expand Down

0 comments on commit cb37a15

Please sign in to comment.