From ea00a0843eb7a7bb074625a663ca4f5c86b8c5bd Mon Sep 17 00:00:00 2001 From: David Wobrock Date: Sat, 16 Oct 2021 11:20:33 +0200 Subject: [PATCH] [4.0.x] Fixed #31503 -- Made autodetector remove unique/index_together before altering fields. Backport of 0314593fe8e7dc685bbb6585eee40e755588864e from main --- django/db/migrations/autodetector.py | 53 ++++++++-- tests/migrations/test_autodetector.py | 147 +++++++++++++++++++++++--- 2 files changed, 181 insertions(+), 19 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 594658ce99..96cb463848 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -178,8 +178,12 @@ class MigrationAutodetector: # 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() @@ -1105,8 +1109,7 @@ class MigrationAutodetector: 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] @@ -1134,13 +1137,49 @@ class MigrationAutodetector: 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, ) diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 6dd4bea4e2..67cc39dec7 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -1564,9 +1564,26 @@ class AutodetectorTests(TestCase): ) # 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): """ @@ -1613,10 +1630,100 @@ class AutodetectorTests(TestCase): ) # 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 are 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]) + + 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_partly_alter_foo_together(self): + initial_author = ModelState('testapp', 'Author', [ + ('id', models.AutoField(primary_key=True)), + ('name', models.CharField(max_length=200)), + ('age', models.IntegerField()), + ], { + 'unique_together': {('name',), ('age',)}, + 'index_together': {('name',)}, + }) + author_reversed_constraints = ModelState('testapp', 'Author', [ + ('id', models.AutoField(primary_key=True)), + ('name', models.CharField(max_length=200)), + ('age', models.IntegerField()), + ], { + 'unique_together': {('age',)}, + 'index_together': {('name',), ('age',)}, + }) + changes = self.get_changes([initial_author], [author_reversed_constraints]) + + self.assertNumberMigrations(changes, 'testapp', 1) + self.assertOperationTypes(changes, 'testapp', 0, [ + 'AlterUniqueTogether', + 'AlterIndexTogether', + ]) + self.assertOperationAttributes( + changes, 'testapp', 0, 0, name='author', unique_together={('age',)}, + ) + self.assertOperationAttributes( + changes, 'testapp', 0, 1, name='author', + index_together={('name',), ('age',)}, + ) def test_rename_field_and_foo_together(self): """ @@ -1629,11 +1736,27 @@ class AutodetectorTests(TestCase): ) # 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={ - ("title", "newfield2") - }) - self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", index_together={("title", "newfield2")}) + 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, 4, name='book', + index_together={('title', 'newfield2')}, + ) def test_proxy(self): """The autodetector correctly deals with proxy models."""