diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index a5646dc377..87f8847071 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -3,7 +3,6 @@ import re from collections import defaultdict, namedtuple from enum import Enum from graphlib import TopologicalSorter -from itertools import chain from django.conf import settings from django.db import models @@ -150,23 +149,17 @@ class MigrationAutodetector: # proxy models and ignoring unmigrated apps. self.old_model_keys = set() self.old_proxy_keys = set() - self.old_unmanaged_keys = set() self.new_model_keys = set() self.new_proxy_keys = set() - self.new_unmanaged_keys = set() for (app_label, model_name), model_state in self.from_state.models.items(): - if not model_state.options.get("managed", True): - self.old_unmanaged_keys.add((app_label, model_name)) - elif app_label not in self.from_state.real_apps: + if app_label not in self.from_state.real_apps: if model_state.options.get("proxy"): self.old_proxy_keys.add((app_label, model_name)) else: self.old_model_keys.add((app_label, model_name)) for (app_label, model_name), model_state in self.to_state.models.items(): - if not model_state.options.get("managed", True): - self.new_unmanaged_keys.add((app_label, model_name)) - elif app_label not in self.from_state.real_apps or ( + if app_label not in self.from_state.real_apps or ( convert_apps and app_label in convert_apps ): if model_state.options.get("proxy"): @@ -235,18 +228,17 @@ class MigrationAutodetector: """ self.kept_model_keys = self.old_model_keys & self.new_model_keys self.kept_proxy_keys = self.old_proxy_keys & self.new_proxy_keys - self.kept_unmanaged_keys = self.old_unmanaged_keys & self.new_unmanaged_keys self.through_users = {} self.old_field_keys = { (app_label, model_name, field_name) - for app_label, model_name in self.kept_model_keys | self.kept_unmanaged_keys + for app_label, model_name in self.kept_model_keys for field_name in self.from_state.models[ app_label, self.renamed_models.get((app_label, model_name), model_name) ].fields } self.new_field_keys = { (app_label, model_name, field_name) - for app_label, model_name in self.kept_model_keys | self.kept_unmanaged_keys + for app_label, model_name in self.kept_model_keys for field_name in self.to_state.models[app_label, model_name].fields } @@ -641,12 +633,10 @@ class MigrationAutodetector: Defer any model options that refer to collections of fields that might be deferred (e.g. unique_together). """ - old_keys = self.old_model_keys | self.old_unmanaged_keys + old_keys = self.old_model_keys added_models = self.new_model_keys - old_keys - added_unmanaged_models = self.new_unmanaged_keys - old_keys - all_added_models = chain( - sorted(added_models, key=self.swappable_first_key, reverse=True), - sorted(added_unmanaged_models, key=self.swappable_first_key, reverse=True), + all_added_models = sorted( + added_models, key=self.swappable_first_key, reverse=True ) for app_label, model_name in all_added_models: model_state = self.to_state.models[app_label, model_name] @@ -740,10 +730,6 @@ class MigrationAutodetector: beginning=True, ) - # Don't add operations which modify the database for unmanaged models - if not model_state.options.get("managed", True): - continue - # Generate operations for each related field for name, field in sorted(related_fields.items()): dependencies = self._get_dependencies_for_foreign_key( @@ -896,20 +882,16 @@ class MigrationAutodetector: def generate_deleted_models(self): """ - Find all deleted models (managed and unmanaged) and make delete - operations for them as well as separate operations to delete any - foreign key or M2M relationships (these are optimized later, if - possible). + Find all deleted models and make delete operations for them as well + as separate operations to delete any foreign key or M2M relationships + (these are optimized later, if possible). Also bring forward removal of any model options that refer to collections of fields - the inverse of generate_created_models(). """ - new_keys = self.new_model_keys | self.new_unmanaged_keys + new_keys = self.new_model_keys deleted_models = self.old_model_keys - new_keys - deleted_unmanaged_models = self.old_unmanaged_keys - new_keys - all_deleted_models = chain( - sorted(deleted_models), sorted(deleted_unmanaged_models) - ) + all_deleted_models = sorted(deleted_models) for app_label, model_name in all_deleted_models: model_state = self.from_state.models[app_label, model_name] # Gather related fields @@ -1681,9 +1663,7 @@ class MigrationAutodetector: self._generate_altered_foo_together(operations.AlterUniqueTogether) def generate_altered_db_table(self): - models_to_check = self.kept_model_keys.union( - self.kept_proxy_keys, self.kept_unmanaged_keys - ) + models_to_check = self.kept_model_keys.union(self.kept_proxy_keys) for app_label, model_name in sorted(models_to_check): old_model_name = self.renamed_models.get( (app_label, model_name), model_name @@ -1702,9 +1682,7 @@ class MigrationAutodetector: ) def generate_altered_db_table_comment(self): - models_to_check = self.kept_model_keys.union( - self.kept_proxy_keys, self.kept_unmanaged_keys - ) + models_to_check = self.kept_model_keys.union(self.kept_proxy_keys) for app_label, model_name in sorted(models_to_check): old_model_name = self.renamed_models.get( (app_label, model_name), model_name @@ -1731,11 +1709,6 @@ class MigrationAutodetector: """ models_to_check = self.kept_model_keys.union( self.kept_proxy_keys, - self.kept_unmanaged_keys, - # unmanaged converted to managed - self.old_unmanaged_keys & self.new_model_keys, - # managed converted to unmanaged - self.old_model_keys & self.new_unmanaged_keys, ) for app_label, model_name in sorted(models_to_check): diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index c60fa3e6b9..1966a2df60 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -712,6 +712,41 @@ class AutodetectorTests(BaseAutodetectorTests): author_unmanaged = ModelState( "testapp", "AuthorUnmanaged", [], {"managed": False}, ("testapp.author",) ) + author_unmanaged_empty = ModelState( + "testapp", + "Author", + [("id", models.AutoField(primary_key=True))], + {"managed": False}, + ("testapp.author",), + ) + author_unmanaged_name_check_constraint = ModelState( + "testapp", + "Author", + [ + ("id", models.AutoField(primary_key=True)), + ("name", models.CharField(max_length=200, default="Ada Lovelace")), + ], + { + "managed": False, + "constraints": [ + models.CheckConstraint( + condition=models.Q(name__contains="Bob"), name="name_contains_bob" + ) + ], + }, + ("testapp.author",), + ) + author_unmanaged_with_book = ModelState( + "testapp", + "Author", + [ + ("id", models.AutoField(primary_key=True)), + ("name", models.CharField(max_length=200)), + ("book", models.ForeignKey("otherapp.Book", models.CASCADE)), + ], + {"managed": False}, + ("testapp.author",), + ) author_unmanaged_name_default = ModelState( "testapp", "Author", @@ -732,6 +767,17 @@ class AutodetectorTests(BaseAutodetectorTests): {"managed": False}, ("testapp.author",), ) + author_unmanaged_renamed_with_book = ModelState( + "testapp", + "Writer", + [ + ("id", models.AutoField(primary_key=True)), + ("name", models.CharField(max_length=200)), + ("book", models.ForeignKey("otherapp.Book", models.CASCADE)), + ], + {"managed": False}, + ("testapp.author",), + ) author_unmanaged_managed = ModelState( "testapp", "AuthorUnmanaged", [], {}, ("testapp.author",) ) @@ -842,6 +888,17 @@ class AutodetectorTests(BaseAutodetectorTests): ], {"db_table": "author_three"}, ) + book_unmanaged = ModelState( + "otherapp", + "Book", + [ + ("id", models.AutoField(primary_key=True)), + ("author", models.ForeignKey("testapp.Author", models.CASCADE)), + ("title", models.CharField(max_length=200)), + ], + {"managed": False}, + ("otherapp.book",), + ) contract = ModelState( "testapp", "Contract", @@ -981,6 +1038,17 @@ class AutodetectorTests(BaseAutodetectorTests): ("title", models.CharField(max_length=200)), ], ) + book_with_author_unmanaged_renamed = ModelState( + "otherapp", + "Book", + [ + ("id", models.AutoField(primary_key=True)), + ("author", models.ForeignKey("testapp.Writer", models.CASCADE)), + ("title", models.CharField(max_length=200)), + ], + {"managed": False}, + ("otherapp.book",), + ) book_with_field_and_author_renamed = ModelState( "otherapp", "Book", @@ -3539,6 +3607,43 @@ class AutodetectorTests(BaseAutodetectorTests): ) self.assertEqual(fk_field.remote_field.model, "testapp.AAuthorProxyProxy") + def test_unmanaged_add_constraints(self): + """Test change detection of new constraints.""" + changes = self.get_changes( + [self.author_unmanaged_name_default], + [self.author_unmanaged_name_check_constraint], + ) + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes(changes, "testapp", 0, ["AddConstraint"]) + added_constraint = models.CheckConstraint( + condition=models.Q(name__contains="Bob"), name="name_contains_bob" + ) + self.assertOperationAttributes( + changes, "testapp", 0, 0, model_name="author", constraint=added_constraint + ) + + def test_unmanaged_add_field(self): + """Tests autodetection of new fields.""" + changes = self.get_changes( + [self.author_unmanaged_empty], [self.author_unmanaged_name_default] + ) + # Right number/type of migrations? + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes(changes, "testapp", 0, ["AddField"]) + self.assertOperationAttributes(changes, "testapp", 0, 0, name="name") + + def test_unmanaged_alter_field(self): + """Tests autodetection of new fields on an unmanaged model.""" + changes = self.get_changes( + [self.author_unmanaged_name_default], [self.author_unmanaged_name_longer] + ) + # Right number/type of migrations? + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes(changes, "testapp", 0, ["AlterField"]) + self.assertOperationAttributes( + changes, "testapp", 0, 0, name="name", preserve_default=True + ) + def test_unmanaged_create(self): """The autodetector correctly deals with unmanaged models.""" # First, we test adding an unmanaged model @@ -3552,18 +3657,50 @@ class AutodetectorTests(BaseAutodetectorTests): changes, "testapp", 0, 0, name="AuthorUnmanaged", options={"managed": False} ) - def test_unmanaged_alter_field(self): - """Tests autodetection of new fields on an unmanaged model.""" + def test_unmanaged_remove_field(self): + """Tests autodetection of removed fields.""" changes = self.get_changes( - [self.author_unmanaged_name_default], [self.author_unmanaged_name_longer] + [self.author_unmanaged_name_default], [self.author_unmanaged_empty] ) # Right number/type of migrations? self.assertNumberMigrations(changes, "testapp", 1) - self.assertOperationTypes(changes, "testapp", 0, ["AlterField"]) + self.assertOperationTypes(changes, "testapp", 0, ["RemoveField"]) self.assertOperationAttributes( - changes, "testapp", 0, 0, name="name", preserve_default=True + changes, "testapp", 0, 0, name="name", model_name="author" ) + def test_unmanaged_remove_constraints(self): + """Test change detection of new constraints.""" + changes = self.get_changes( + [self.author_unmanaged_name_check_constraint], + [self.author_unmanaged_name_default], + ) + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes(changes, "testapp", 0, ["RemoveConstraint"]) + self.assertOperationAttributes( + changes, "testapp", 0, 0, model_name="author", name="name_contains_bob" + ) + + def test_unmanaged_rename_model(self): + """Tests autodetection of renamed models.""" + changes = self.get_changes( + [self.author_unmanaged_with_book, self.book_unmanaged], + [ + self.author_unmanaged_renamed_with_book, + self.book_with_author_unmanaged_renamed, + ], + MigrationQuestioner({"ask_rename_model": True}), + ) + # Right number/type of migrations? + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes(changes, "testapp", 0, ["RenameModel"]) + self.assertOperationAttributes( + changes, "testapp", 0, 0, old_name="Author", new_name="Writer" + ) + # Now that RenameModel handles related fields too, there should be + # no AlterField for the related field. + self.assertNumberMigrations(changes, "otherapp", 0) + def test_unmanaged_delete(self): changes = self.get_changes( [self.author_empty, self.author_unmanaged], [self.author_empty]