Browse Source

Refs #27236 -- Added generic mechanism to handle the deprecation of migration operations.

David Wobrock 2 years ago
parent
commit
41019e48bb

+ 1 - 0
django/core/checks/__init__.py

@@ -19,6 +19,7 @@ import django.core.checks.caches  # NOQA isort:skip
 import django.core.checks.compatibility.django_4_0  # NOQA isort:skip
 import django.core.checks.database  # NOQA isort:skip
 import django.core.checks.files  # NOQA isort:skip
+import django.core.checks.migrations  # NOQA isort:skip
 import django.core.checks.model_checks  # NOQA isort:skip
 import django.core.checks.security.base  # NOQA isort:skip
 import django.core.checks.security.csrf  # NOQA isort:skip

+ 12 - 0
django/core/checks/migrations.py

@@ -0,0 +1,12 @@
+from django.core.checks import Tags, register
+
+
+@register(Tags.migrations)
+def check_migration_operations(**kwargs):
+    from django.db.migrations.loader import MigrationLoader
+
+    errors = []
+    loader = MigrationLoader(None, ignore_no_migrations=True)
+    for migration in loader.disk_migrations.values():
+        errors.extend(migration.check())
+    return errors

+ 1 - 0
django/core/checks/registry.py

@@ -15,6 +15,7 @@ class Tags:
     compatibility = "compatibility"
     database = "database"
     files = "files"
+    migrations = "migrations"
     models = "models"
     security = "security"
     signals = "signals"

+ 6 - 0
django/db/migrations/migration.py

@@ -219,6 +219,12 @@ class Migration:
             name = new_name
         return name
 
+    def check(self):
+        errors = []
+        for operation in self.operations:
+            errors.extend(operation.check_deprecation_details())
+        return errors
+
 
 class SwappableTuple(tuple):
     """

+ 4 - 1
django/db/migrations/operations/base.py

@@ -1,7 +1,8 @@
 from django.db import router
+from django.utils.deprecation import DeprecationForHistoricalMigrationMixin
 
 
-class Operation:
+class Operation(DeprecationForHistoricalMigrationMixin):
     """
     Base class for migration operations.
 
@@ -33,6 +34,8 @@ class Operation:
 
     serialization_expand_args = []
 
+    check_type = "migrations"
+
     def __new__(cls, *args, **kwargs):
         # We capture the arguments to make returning them trivial
         self = object.__new__(cls)

+ 5 - 0
docs/ref/checks.txt

@@ -83,6 +83,7 @@ Django's system checks are organized using the following tags:
   you specify configured database aliases using the ``--database`` option when
   calling the :djadmin:`check` command.
 * ``files``: Checks files related configuration.
+* ``migrations``: Checks of migration operations.
 * ``models``: Checks of model, field, and manager definitions.
 * ``security``: Checks security related configuration.
 * ``signals``: Checks on signal declarations and handler registrations.
@@ -94,6 +95,10 @@ Django's system checks are organized using the following tags:
 
 Some checks may be registered with multiple tags.
 
+.. versionchanged:: 4.2
+
+    The ``migrations`` tag was added.
+
 Core system checks
 ==================
 

+ 2 - 1
docs/releases/4.2.txt

@@ -169,7 +169,8 @@ Management Commands
 Migrations
 ~~~~~~~~~~
 
-* ...
+* The new :ref:`generic mechanism <migrations-removing-operation>` allows
+  handling the deprecation of migration operations.
 
 Models
 ~~~~~~

+ 12 - 8
docs/topics/checks.txt

@@ -128,18 +128,18 @@ The code below is equivalent to the code above::
 
 .. _field-checking:
 
-Field, model, manager, and database checks
-------------------------------------------
+Field, model, manager, migration, and database checks
+-----------------------------------------------------
 
 In some cases, you won't need to register your check function -- you can
 piggyback on an existing registration.
 
-Fields, models, model managers, and database backends all implement a
-``check()`` method that is already registered with the check framework. If you
-want to add extra checks, you can extend the implementation on the base class,
-perform any extra checks you need, and append any messages to those generated
-by the base class. It's recommended that you delegate each check to separate
-methods.
+Fields, models, model managers, migrations, and database backends all implement
+a ``check()`` method that is already registered with the check framework. If
+you want to add extra checks, you can extend the implementation on the base
+class, perform any extra checks you need, and append any messages to those
+generated by the base class. It's recommended that you delegate each check to
+separate methods.
 
 Consider an example where you are implementing a custom field named
 ``RangedIntegerField``. This field adds ``min`` and ``max`` arguments to the
@@ -194,6 +194,10 @@ the only difference is that the check is a classmethod, not an instance method::
             # ... your own checks ...
             return errors
 
+.. versionchanged:: 4.2
+
+    Migration checks were added.
+
 Writing tests
 -------------
 

+ 50 - 0
docs/topics/migrations.txt

@@ -503,6 +503,56 @@ database migrations such as ``__init__()``, ``deconstruct()``, and
 which reference the field exist. For example, after squashing migrations and
 removing the old ones, you should be able to remove the field completely.
 
+.. _migrations-removing-operation:
+
+Considerations when removing migration operations
+=================================================
+
+.. versionadded:: 4.2
+
+Removing custom operations from your project or third-party app will cause a
+problem if they are referenced in old migrations.
+
+To help with this situation, Django provides some operation attributes to
+assist with operation deprecation using the :doc:`system checks framework
+</topics/checks>`.
+
+Add the ``system_check_deprecated_details`` attribute to your operation similar
+to the following::
+
+    class MyCustomOperation(Operation):
+        system_check_deprecated_details = {
+            "msg": (
+                "MyCustomOperation has been deprecated. Support for it "
+                "(except in historical migrations) will be removed in "
+                "Django 5.1."
+            ),
+            "hint": "Use DifferentOperation instead.",  # optional
+            "id": "migrations.W900",  # pick a unique ID for your operation.
+        }
+
+After a deprecation period of your choosing (two or three feature releases for
+operations in Django itself), change the ``system_check_deprecated_details``
+attribute to ``system_check_removed_details`` and update the dictionary similar
+to::
+
+    class MyCustomOperation(Operation):
+        system_check_removed_details = {
+            "msg": (
+                "MyCustomOperation has been removed except for support in "
+                "historical migrations."
+            ),
+            "hint': "Use DifferentOperation instead.",
+            "id": "migrations.E900",  # pick a unique ID for your operation.
+        }
+
+You should keep the operation's methods that are required for it to operate in
+database migrations such as ``__init__()``, ``state_forwards()``,
+``database_forwards()``, and ``database_backwards()``. Keep this stub operation
+for as long as any migrations which reference the operation exist. For example,
+after squashing migrations and removing the old ones, you should be able to
+remove the operation completely.
+
 .. _data-migrations:
 
 Data Migrations

+ 101 - 0
tests/check_framework/test_migrations.py

@@ -0,0 +1,101 @@
+from django.core import checks
+from django.db import migrations
+from django.db.migrations.operations.base import Operation
+from django.test import TestCase
+
+
+class DeprecatedMigrationOperationTests(TestCase):
+    def test_default_operation(self):
+        class MyOperation(Operation):
+            system_check_deprecated_details = {}
+
+        my_operation = MyOperation()
+
+        class Migration(migrations.Migration):
+            operations = [my_operation]
+
+        self.assertEqual(
+            Migration("name", "app_label").check(),
+            [
+                checks.Warning(
+                    msg="MyOperation has been deprecated.",
+                    obj=my_operation,
+                    id="migrations.WXXX",
+                )
+            ],
+        )
+
+    def test_user_specified_details(self):
+        class MyOperation(Operation):
+            system_check_deprecated_details = {
+                "msg": "This operation is deprecated and will be removed soon.",
+                "hint": "Use something else.",
+                "id": "migrations.W999",
+            }
+
+        my_operation = MyOperation()
+
+        class Migration(migrations.Migration):
+            operations = [my_operation]
+
+        self.assertEqual(
+            Migration("name", "app_label").check(),
+            [
+                checks.Warning(
+                    msg="This operation is deprecated and will be removed soon.",
+                    obj=my_operation,
+                    hint="Use something else.",
+                    id="migrations.W999",
+                )
+            ],
+        )
+
+
+class RemovedMigrationOperationTests(TestCase):
+    def test_default_operation(self):
+        class MyOperation(Operation):
+            system_check_removed_details = {}
+
+        my_operation = MyOperation()
+
+        class Migration(migrations.Migration):
+            operations = [my_operation]
+
+        self.assertEqual(
+            Migration("name", "app_label").check(),
+            [
+                checks.Error(
+                    msg=(
+                        "MyOperation has been removed except for support in historical "
+                        "migrations."
+                    ),
+                    obj=my_operation,
+                    id="migrations.EXXX",
+                )
+            ],
+        )
+
+    def test_user_specified_details(self):
+        class MyOperation(Operation):
+            system_check_removed_details = {
+                "msg": "Support for this operation is gone.",
+                "hint": "Use something else.",
+                "id": "migrations.E999",
+            }
+
+        my_operation = MyOperation()
+
+        class Migration(migrations.Migration):
+            operations = [my_operation]
+
+        self.assertEqual(
+            Migration("name", "app_label").check(),
+            [
+                checks.Error(
+                    msg="Support for this operation is gone.",
+                    obj=my_operation,
+                    hint="Use something else.",
+                    id="migrations.E999",
+                )
+            ],
+        )

+ 14 - 3
tests/db_functions/migrations/0001_setup_extensions.py

@@ -1,11 +1,22 @@
-from unittest import mock
-
 from django.db import migrations
+from django.db.migrations.operations.base import Operation
+
+
+class DummyOperation(Operation):
+    def state_forwards(self, app_label, state):
+        pass
+
+    def database_forwards(self, app_label, schema_editor, from_state, to_state):
+        pass
+
+    def database_backwards(self, app_label, schema_editor, from_state, to_state):
+        pass
+
 
 try:
     from django.contrib.postgres.operations import CryptoExtension
 except ImportError:
-    CryptoExtension = mock.Mock()
+    CryptoExtension = DummyOperation
 
 
 class Migration(migrations.Migration):

+ 22 - 11
tests/postgres_tests/migrations/0001_setup_extensions.py

@@ -1,6 +1,17 @@
-from unittest import mock
-
 from django.db import connection, migrations
+from django.db.migrations.operations.base import Operation
+
+
+class DummyOperation(Operation):
+    def state_forwards(self, app_label, state):
+        pass
+
+    def database_forwards(self, app_label, schema_editor, from_state, to_state):
+        pass
+
+    def database_backwards(self, app_label, schema_editor, from_state, to_state):
+        pass
+
 
 try:
     from django.contrib.postgres.operations import (
@@ -15,14 +26,14 @@ try:
         UnaccentExtension,
     )
 except ImportError:
-    BloomExtension = mock.Mock()
-    BtreeGinExtension = mock.Mock()
-    BtreeGistExtension = mock.Mock()
-    CITextExtension = mock.Mock()
-    CreateExtension = mock.Mock()
-    HStoreExtension = mock.Mock()
-    TrigramExtension = mock.Mock()
-    UnaccentExtension = mock.Mock()
+    BloomExtension = DummyOperation
+    BtreeGinExtension = DummyOperation
+    BtreeGistExtension = DummyOperation
+    CITextExtension = DummyOperation
+    CreateExtension = DummyOperation
+    HStoreExtension = DummyOperation
+    TrigramExtension = DummyOperation
+    UnaccentExtension = DummyOperation
     needs_crypto_extension = False
 else:
     needs_crypto_extension = (
@@ -41,7 +52,7 @@ class Migration(migrations.Migration):
         # dash in its name.
         CreateExtension("uuid-ossp"),
         # CryptoExtension is required for RandomUUID() on PostgreSQL < 13.
-        CryptoExtension() if needs_crypto_extension else mock.Mock(),
+        CryptoExtension() if needs_crypto_extension else DummyOperation(),
         HStoreExtension(),
         TrigramExtension(),
         UnaccentExtension(),