From 41019e48bbf082c985e6ba3bad34d118b903bff1 Mon Sep 17 00:00:00 2001 From: David Wobrock Date: Sat, 2 Jul 2022 19:55:37 +0200 Subject: [PATCH] Refs #27236 -- Added generic mechanism to handle the deprecation of migration operations. --- django/core/checks/__init__.py | 1 + django/core/checks/migrations.py | 12 +++ django/core/checks/registry.py | 1 + django/db/migrations/migration.py | 6 ++ django/db/migrations/operations/base.py | 5 +- docs/ref/checks.txt | 5 + docs/releases/4.2.txt | 3 +- docs/topics/checks.txt | 20 ++-- docs/topics/migrations.txt | 50 +++++++++ tests/check_framework/test_migrations.py | 101 ++++++++++++++++++ .../migrations/0001_setup_extensions.py | 17 ++- .../migrations/0001_setup_extensions.py | 33 ++++-- 12 files changed, 230 insertions(+), 24 deletions(-) create mode 100644 django/core/checks/migrations.py create mode 100644 tests/check_framework/test_migrations.py diff --git a/django/core/checks/__init__.py b/django/core/checks/__init__.py index 998ab9dee2..deea0d471a 100644 --- a/django/core/checks/__init__.py +++ b/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 diff --git a/django/core/checks/migrations.py b/django/core/checks/migrations.py new file mode 100644 index 0000000000..92649a59a9 --- /dev/null +++ b/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 diff --git a/django/core/checks/registry.py b/django/core/checks/registry.py index f4bdea8691..328209130a 100644 --- a/django/core/checks/registry.py +++ b/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" diff --git a/django/db/migrations/migration.py b/django/db/migrations/migration.py index ea9d02a94a..7cc3e22361 100644 --- a/django/db/migrations/migration.py +++ b/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): """ diff --git a/django/db/migrations/operations/base.py b/django/db/migrations/operations/base.py index 7d4dff2597..40128174c7 100644 --- a/django/db/migrations/operations/base.py +++ b/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) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 92278db659..55e4ca64b2 100644 --- a/docs/ref/checks.txt +++ b/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 ================== diff --git a/docs/releases/4.2.txt b/docs/releases/4.2.txt index 34f8362be9..6282bfa580 100644 --- a/docs/releases/4.2.txt +++ b/docs/releases/4.2.txt @@ -169,7 +169,8 @@ Management Commands Migrations ~~~~~~~~~~ -* ... +* The new :ref:`generic mechanism ` allows + handling the deprecation of migration operations. Models ~~~~~~ diff --git a/docs/topics/checks.txt b/docs/topics/checks.txt index 9586466285..0864d6db38 100644 --- a/docs/topics/checks.txt +++ b/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 ------------- diff --git a/docs/topics/migrations.txt b/docs/topics/migrations.txt index cf3451a782..2350ac756c 100644 --- a/docs/topics/migrations.txt +++ b/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 +`. + +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 diff --git a/tests/check_framework/test_migrations.py b/tests/check_framework/test_migrations.py new file mode 100644 index 0000000000..0b00690e77 --- /dev/null +++ b/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", + ) + ], + ) diff --git a/tests/db_functions/migrations/0001_setup_extensions.py b/tests/db_functions/migrations/0001_setup_extensions.py index 0289055499..5909a96eb8 100644 --- a/tests/db_functions/migrations/0001_setup_extensions.py +++ b/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): diff --git a/tests/postgres_tests/migrations/0001_setup_extensions.py b/tests/postgres_tests/migrations/0001_setup_extensions.py index 090abf9649..3edeff7b82 100644 --- a/tests/postgres_tests/migrations/0001_setup_extensions.py +++ b/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(),