Commit 061caa5b authored by Tim Graham's avatar Tim Graham
Browse files

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.
parent 69ee7c8d

django/db/migrations/autodetector.py

+18 −11
Original line number Diff line number Diff line
@@ -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]

django/db/migrations/operations/models.py

+1 −0
Original line number Diff line number Diff line
@@ -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",

docs/releases/1.7.2.txt

+5 −0
Original line number Diff line number Diff line
@@ -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`).

tests/migrations/test_autodetector.py

+19 −6
Original line number Diff line number Diff line
@@ -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):

        """