From 061caa5b386681dc7bdef16918873043224a299c Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Tue, 23 Dec 2014 12:27:49 -0500 Subject: [PATCH] Fixed #24037 -- Prevented data loss possibility when changing Meta.managed. The migrations autodetector now issues AlterModelOptions operations for Meta.managed changes instead of DeleteModel + CreateModel. Thanks iambibhas for the report and Simon and Markus for review. --- django/db/migrations/autodetector.py | 29 ++++++++++++++--------- django/db/migrations/operations/models.py | 1 + docs/releases/1.7.2.txt | 5 ++++ tests/migrations/test_autodetector.py | 25 ++++++++++++++----- 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index b361304c65..080e53182e 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -447,8 +447,9 @@ class MigrationAutodetector(object): We also defer any model options that refer to collections of fields that might be deferred (e.g. unique_together, index_together). """ - added_models = set(self.new_model_keys) - set(self.old_model_keys) - added_unmanaged_models = set(self.new_unmanaged_keys) - set(self.old_unmanaged_keys) + old_keys = set(self.old_model_keys).union(self.old_unmanaged_keys) + added_models = set(self.new_model_keys) - old_keys + added_unmanaged_models = set(self.new_unmanaged_keys) - old_keys models = chain( sorted(added_models, key=self.swappable_first_key, reverse=True), sorted(added_unmanaged_models, key=self.swappable_first_key, reverse=True) @@ -625,19 +626,14 @@ class MigrationAutodetector(object): We also bring forward removal of any model options that refer to collections of fields - the inverse of generate_created_models(). """ - deleted_models = set(self.old_model_keys) - set(self.new_model_keys) - deleted_unmanaged_models = set(self.old_unmanaged_keys) - set(self.new_unmanaged_keys) + new_keys = set(self.new_model_keys).union(self.new_unmanaged_keys) + deleted_models = set(self.old_model_keys) - new_keys + deleted_unmanaged_models = set(self.old_unmanaged_keys) - new_keys models = chain(sorted(deleted_models), sorted(deleted_unmanaged_models)) for app_label, model_name in models: model_state = self.from_state.models[app_label, model_name] model = self.old_apps.get_model(app_label, model_name) if not model._meta.managed: - self.add_operation( - app_label, - operations.DeleteModel( - name=model_state.name, - ), - ) # Skip here, no need to handle fields for unmanaged models continue @@ -947,7 +943,18 @@ class MigrationAutodetector(object): makes an operation to represent them in state changes (in case Python code in migrations needs them) """ - models_to_check = self.kept_model_keys.union(self.kept_proxy_keys).union(self.kept_unmanaged_keys) + models_to_check = self.kept_model_keys.union( + self.kept_proxy_keys + ).union( + self.kept_unmanaged_keys + ).union( + # unmanaged converted to managed + set(self.old_unmanaged_keys).intersection(self.new_model_keys) + ).union( + # managed converted to unmanaged + set(self.old_model_keys).intersection(self.new_unmanaged_keys) + ) + for app_label, model_name in sorted(models_to_check): 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] diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index d6b6275da3..2436701919 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -431,6 +431,7 @@ class AlterModelOptions(Operation): # Model options we want to compare and preserve in an AlterModelOptions op ALTER_OPTION_KEYS = [ "get_latest_by", + "managed", "ordering", "permissions", "default_permissions", diff --git a/docs/releases/1.7.2.txt b/docs/releases/1.7.2.txt index 179bd5f7db..309aa736ad 100644 --- a/docs/releases/1.7.2.txt +++ b/docs/releases/1.7.2.txt @@ -162,3 +162,8 @@ Bugfixes * Added ``datetime.time`` support to migrations questioner (:ticket:`23998`). * Fixed admindocs crash on apps installed as eggs (:ticket:`23525`). + +* Changed migrations autodetector to generate an ``AlterModelOptions`` operation + instead of ``DeleteModel`` and ``CreateModel`` operations when changing + ``Meta.managed``. This prevents data loss when changing ``managed`` from + ``False`` to ``True`` and vice versa (:ticket:`24037`). diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index aecc8fc2fe..328ccf05db 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -1065,7 +1065,7 @@ class AutodetectorTests(TestCase): # The field name the FK on the book model points to self.assertEqual(changes['otherapp'][0].operations[0].fields[2][1].rel.field_name, 'pk_field') - def test_unmanaged(self): + def test_unmanaged_create(self): """Tests that the autodetector correctly deals with managed models.""" # First, we test adding an unmanaged model before = self.make_project_state([self.author_empty]) @@ -1075,9 +1075,10 @@ class AutodetectorTests(TestCase): # Right number/type of migrations? self.assertNumberMigrations(changes, 'testapp', 1) self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel"]) - self.assertOperationAttributes(changes, 'testapp', 0, 0, name="AuthorUnmanaged") - self.assertEqual(changes['testapp'][0].operations[0].options['managed'], False) + self.assertOperationAttributes(changes, 'testapp', 0, 0, + name="AuthorUnmanaged", options={"managed": False}) + def test_unmanaged_to_managed(self): # Now, we test turning an unmanaged model into a managed model before = self.make_project_state([self.author_empty, self.author_unmanaged]) after = self.make_project_state([self.author_empty, self.author_unmanaged_managed]) @@ -1085,9 +1086,21 @@ class AutodetectorTests(TestCase): changes = autodetector._detect_changes() # Right number/type of migrations? self.assertNumberMigrations(changes, 'testapp', 1) - self.assertOperationTypes(changes, 'testapp', 0, ["DeleteModel", "CreateModel"]) - self.assertOperationAttributes(changes, 'testapp', 0, 0, name="AuthorUnmanaged") - self.assertOperationAttributes(changes, 'testapp', 0, 1, name="AuthorUnmanaged") + self.assertOperationTypes(changes, 'testapp', 0, ["AlterModelOptions"]) + self.assertOperationAttributes(changes, 'testapp', 0, 0, + name="authorunmanaged", options={}) + + def test_managed_to_unmanaged(self): + # Now, we turn managed to unmanaged. + before = self.make_project_state([self.author_empty, self.author_unmanaged_managed]) + after = self.make_project_state([self.author_empty, self.author_unmanaged]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number/type of migrations? + self.assertNumberMigrations(changes, 'testapp', 1) + self.assertOperationTypes(changes, "testapp", 0, ["AlterModelOptions"]) + self.assertOperationAttributes(changes, "testapp", 0, 0, + name="authorunmanaged", options={"managed": False}) def test_unmanaged_custom_pk(self): """