From a6385b382e05a614a99e5a5913d8e631823159a2 Mon Sep 17 00:00:00 2001 From: David Wobrock Date: Tue, 17 May 2022 16:13:35 +0200 Subject: [PATCH] Fixed #27236 -- Deprecated Meta.index_together in favor of Meta.indexes. This also deprecates AlterIndexTogether migration operation. --- django/db/migrations/operations/models.py | 8 +++++ django/db/models/options.py | 8 +++++ docs/internals/deprecation.txt | 5 +++ docs/ref/checks.txt | 6 ++++ docs/ref/migration-operations.txt | 7 ++++ docs/ref/models/options.txt | 10 +++--- docs/releases/4.2.txt | 32 +++++++++++++++++++ .../migrations_test_apps/__init__.py | 0 .../index_together_app/__init__.py | 0 .../index_together_app/apps.py | 5 +++ .../migrations/0001_initial.py | 16 ++++++++++ .../index_together_app/migrations/__init__.py | 0 tests/check_framework/test_migrations.py | 21 ++++++++++++ tests/indexes/tests.py | 3 ++ tests/invalid_models_tests/test_models.py | 4 ++- tests/migrations/test_autodetector.py | 4 ++- tests/migrations/test_base.py | 3 +- tests/migrations/test_operations.py | 15 ++++++++- tests/migrations/test_optimizer.py | 3 ++ tests/migrations/test_state.py | 10 ++++-- .../test_index_together_deprecation.py | 20 ++++++++++++ tests/schema/tests.py | 12 ++++++- 22 files changed, 178 insertions(+), 14 deletions(-) create mode 100644 tests/check_framework/migrations_test_apps/__init__.py create mode 100644 tests/check_framework/migrations_test_apps/index_together_app/__init__.py create mode 100644 tests/check_framework/migrations_test_apps/index_together_app/apps.py create mode 100644 tests/check_framework/migrations_test_apps/index_together_app/migrations/0001_initial.py create mode 100644 tests/check_framework/migrations_test_apps/index_together_app/migrations/__init__.py create mode 100644 tests/model_options/test_index_together_deprecation.py diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 75a3b8b030..91a4e14cb7 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -615,6 +615,14 @@ class AlterIndexTogether(AlterTogetherOptionOperation): option_name = "index_together" + system_check_deprecated_details = { + "msg": ( + "AlterIndexTogether is deprecated. Support for it (except in historical " + "migrations) will be removed in Django 5.1." + ), + "id": "migrations.W001", + } + def __init__(self, name, index_together): super().__init__(name, index_together) diff --git a/django/db/models/options.py b/django/db/models/options.py index 3caad38072..938b996d67 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -1,6 +1,7 @@ import bisect import copy import inspect +import warnings from collections import defaultdict from django.apps import apps @@ -10,6 +11,7 @@ from django.db import connections from django.db.models import AutoField, Manager, OrderWrt, UniqueConstraint from django.db.models.query_utils import PathInfo from django.utils.datastructures import ImmutableList, OrderedSet +from django.utils.deprecation import RemovedInDjango51Warning from django.utils.functional import cached_property from django.utils.module_loading import import_string from django.utils.text import camel_case_to_spaces, format_lazy @@ -200,6 +202,12 @@ class Options: self.unique_together = normalize_together(self.unique_together) self.index_together = normalize_together(self.index_together) + if self.index_together: + warnings.warn( + f"'index_together' is deprecated. Use 'Meta.indexes' in " + f"{self.label!r} instead.", + RemovedInDjango51Warning, + ) # App label/class name interpolation for names of constraints and # indexes. if not getattr(cls._meta, "abstract", False): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 5709b45865..160605ddcd 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -17,6 +17,11 @@ details on these changes. * The ``BaseUserManager.make_random_password()`` method will be removed. +* The model's ``Meta.index_together`` option will be removed. + +* The ``AlterIndexTogether`` migration operation will be removed. A stub + operation will remain for compatibility with historical migrations. + .. _deprecation-removed-in-5.0: 5.0 diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 55e4ca64b2..8dbada5787 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -397,6 +397,12 @@ Models * **models.W045**: Check constraint ```` contains ``RawSQL()`` expression and won't be validated during the model ``full_clean()``. +Migrations +---------- + +* **migrations.W001**: ``AlterIndexTogether`` is deprecated. Support for it + (except in historical migrations) will be removed in Django 5.1. + Security -------- diff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt index f8bfc4fb6b..b3d8992738 100644 --- a/docs/ref/migration-operations.txt +++ b/docs/ref/migration-operations.txt @@ -106,6 +106,13 @@ Changes the model's set of custom indexes (the :attr:`~django.db.models.Options.index_together` option on the ``Meta`` subclass). +.. deprecated:: 4.2 + + ``AlterIndexTogether`` is deprecated in favor of + :class:`~django.db.migrations.operations.AddIndex`, + :class:`~django.db.migrations.operations.RemoveIndex`, and + :class:`~django.db.migrations.operations.RenameIndex` operations. + ``AlterOrderWithRespectTo`` --------------------------- diff --git a/docs/ref/models/options.txt b/docs/ref/models/options.txt index e45dfee8dd..a1629168af 100644 --- a/docs/ref/models/options.txt +++ b/docs/ref/models/options.txt @@ -431,12 +431,6 @@ Django quotes column and table names behind the scenes. .. attribute:: Options.index_together - .. admonition:: Use the :attr:`~Options.indexes` option instead. - - The newer :attr:`~Options.indexes` option provides more functionality - than ``index_together``. ``index_together`` may be deprecated in the - future. - Sets of field names that, taken together, are indexed:: index_together = [ @@ -451,6 +445,10 @@ Django quotes column and table names behind the scenes. index_together = ["pub_date", "deadline"] + .. deprecated:: 4.2 + + Use the :attr:`~Options.indexes` option instead. + ``constraints`` --------------- diff --git a/docs/releases/4.2.txt b/docs/releases/4.2.txt index a8de626993..a1d2bb96bb 100644 --- a/docs/releases/4.2.txt +++ b/docs/releases/4.2.txt @@ -275,6 +275,36 @@ Miscellaneous Features deprecated in 4.2 ========================== +``index_together`` option is deprecated in favor of ``indexes`` +--------------------------------------------------------------- + +The :attr:`Meta.index_together ` +option is deprecated in favor of the :attr:`~django.db.models.Options.indexes` +option. + +Migrating existing ``index_together`` should be handled as a migration. For +example:: + + class Author(models.Model): + rank = models.IntegerField() + name = models.CharField(max_length=30) + + class Meta: + index_together = [["rank", "name"]] + +Should become:: + + class Author(models.Model): + ranl = models.IntegerField() + name = models.CharField(max_length=30) + + class Meta: + indexes = [models.Index(fields=["rank", "name"])] + +Running the :djadmin:`makemigrations` command will generate a migration +containing a :class:`~django.db.migrations.operations.RenameIndex` operation +which will rename the existing index. + Miscellaneous ------------- @@ -282,3 +312,5 @@ Miscellaneous `recipes and best practices `_ for using Python's :py:mod:`secrets` module to generate passwords. + +* The ``AlterIndexTogether`` migration operation is deprecated. diff --git a/tests/check_framework/migrations_test_apps/__init__.py b/tests/check_framework/migrations_test_apps/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/check_framework/migrations_test_apps/index_together_app/__init__.py b/tests/check_framework/migrations_test_apps/index_together_app/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/check_framework/migrations_test_apps/index_together_app/apps.py b/tests/check_framework/migrations_test_apps/index_together_app/apps.py new file mode 100644 index 0000000000..3bb7170897 --- /dev/null +++ b/tests/check_framework/migrations_test_apps/index_together_app/apps.py @@ -0,0 +1,5 @@ +from django.apps import AppConfig + + +class IndexTogetherAppConfig(AppConfig): + name = "check_framework.migrations_test_apps.index_together_app" diff --git a/tests/check_framework/migrations_test_apps/index_together_app/migrations/0001_initial.py b/tests/check_framework/migrations_test_apps/index_together_app/migrations/0001_initial.py new file mode 100644 index 0000000000..c642ee6c57 --- /dev/null +++ b/tests/check_framework/migrations_test_apps/index_together_app/migrations/0001_initial.py @@ -0,0 +1,16 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + operations = [ + migrations.CreateModel( + "SimpleModel", + [ + ("field", models.IntegerField()), + ], + ), + migrations.AlterIndexTogether("SimpleModel", index_together=(("id", "field"),)), + ] diff --git a/tests/check_framework/migrations_test_apps/index_together_app/migrations/__init__.py b/tests/check_framework/migrations_test_apps/index_together_app/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/check_framework/test_migrations.py b/tests/check_framework/test_migrations.py index 0b00690e77..e41a68fa54 100644 --- a/tests/check_framework/test_migrations.py +++ b/tests/check_framework/test_migrations.py @@ -1,7 +1,11 @@ +from unittest.mock import ANY + from django.core import checks +from django.core.checks.migrations import check_migration_operations from django.db import migrations from django.db.migrations.operations.base import Operation from django.test import TestCase +from django.test.utils import override_settings class DeprecatedMigrationOperationTests(TestCase): @@ -50,6 +54,23 @@ class DeprecatedMigrationOperationTests(TestCase): ], ) + @override_settings( + INSTALLED_APPS=["check_framework.migrations_test_apps.index_together_app"] + ) + def tests_check_alter_index_together(self): + errors = check_migration_operations() + self.assertEqual( + errors, + [ + checks.Warning( + "AlterIndexTogether is deprecated. Support for it (except in " + "historical migrations) will be removed in Django 5.1.", + obj=ANY, + id="migrations.W001", + ) + ], + ) + class RemovedMigrationOperationTests(TestCase): def test_default_operation(self): diff --git a/tests/indexes/tests.py b/tests/indexes/tests.py index fb7796ddf8..8fdaec25f5 100644 --- a/tests/indexes/tests.py +++ b/tests/indexes/tests.py @@ -16,11 +16,13 @@ from django.db.models.functions import Lower from django.test import ( TestCase, TransactionTestCase, + ignore_warnings, skipIfDBFeature, skipUnlessDBFeature, ) from django.test.utils import isolate_apps, override_settings from django.utils import timezone +from django.utils.deprecation import RemovedInDjango51Warning from .models import Article, ArticleTranslation, IndexedArticle2 @@ -78,6 +80,7 @@ class SchemaIndexesTests(TestCase): index_sql[0], ) + @ignore_warnings(category=RemovedInDjango51Warning) @isolate_apps("indexes") def test_index_together_single_list(self): class IndexTogetherSingleList(Model): diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py index 08aca62850..5a5aeccdf5 100644 --- a/tests/invalid_models_tests/test_models.py +++ b/tests/invalid_models_tests/test_models.py @@ -5,8 +5,9 @@ from django.core.checks.model_checks import _check_lazy_references from django.db import connection, connections, models from django.db.models.functions import Abs, Lower, Round from django.db.models.signals import post_init -from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature +from django.test import SimpleTestCase, TestCase, ignore_warnings, skipUnlessDBFeature from django.test.utils import isolate_apps, override_settings, register_lookup +from django.utils.deprecation import RemovedInDjango51Warning class EmptyRouter: @@ -29,6 +30,7 @@ def get_max_column_name_length(): @isolate_apps("invalid_models_tests") +@ignore_warnings(category=RemovedInDjango51Warning) class IndexTogetherTests(SimpleTestCase): def test_non_iterable(self): class Model(models.Model): diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 09da03c9fd..4911e41150 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -12,8 +12,9 @@ from django.db.migrations.graph import MigrationGraph from django.db.migrations.loader import MigrationLoader from django.db.migrations.questioner import MigrationQuestioner from django.db.migrations.state import ModelState, ProjectState -from django.test import SimpleTestCase, TestCase, override_settings +from django.test import SimpleTestCase, TestCase, ignore_warnings, override_settings from django.test.utils import isolate_lru_cache +from django.utils.deprecation import RemovedInDjango51Warning from .models import FoodManager, FoodQuerySet @@ -4588,6 +4589,7 @@ class AutodetectorTests(BaseAutodetectorTests): self.assertOperationAttributes(changes, "testapp", 0, 0, name="Book") +@ignore_warnings(category=RemovedInDjango51Warning) class AutodetectorIndexTogetherTests(BaseAutodetectorTests): book_index_together = ModelState( "otherapp", diff --git a/tests/migrations/test_base.py b/tests/migrations/test_base.py index cf9dd029bb..3f1559b8d6 100644 --- a/tests/migrations/test_base.py +++ b/tests/migrations/test_base.py @@ -255,7 +255,7 @@ class OperationTestBase(MigrationTestBase): unique_together=False, options=False, db_table=None, - index_together=False, + index_together=False, # RemovedInDjango51Warning. constraints=None, indexes=None, ): @@ -263,6 +263,7 @@ class OperationTestBase(MigrationTestBase): # Make the "current" state. model_options = { "swappable": "TEST_SWAP_MODEL", + # RemovedInDjango51Warning. "index_together": [["weight", "pink"]] if index_together else [], "unique_together": [["pink", "weight"]] if unique_together else [], } diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 890af778db..d60f5cd183 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -5,8 +5,14 @@ from django.db.migrations.operations.fields import FieldOperation from django.db.migrations.state import ModelState, ProjectState from django.db.models.functions import Abs from django.db.transaction import atomic -from django.test import SimpleTestCase, override_settings, skipUnlessDBFeature +from django.test import ( + SimpleTestCase, + ignore_warnings, + override_settings, + skipUnlessDBFeature, +) from django.test.utils import CaptureQueriesContext +from django.utils.deprecation import RemovedInDjango51Warning from .models import FoodManager, FoodQuerySet, UnicodeModel from .test_base import OperationTestBase @@ -2485,6 +2491,7 @@ class OperationTests(OperationTestBase): atomic=connection.features.supports_atomic_references_rename, ) + @ignore_warnings(category=RemovedInDjango51Warning) def test_rename_field(self): """ Tests the RenameField operation. @@ -2556,6 +2563,7 @@ class OperationTests(OperationTestBase): self.assertColumnExists("test_rnflut_pony", "pink") self.assertColumnNotExists("test_rnflut_pony", "blue") + @ignore_warnings(category=RemovedInDjango51Warning) def test_rename_field_index_together(self): project_state = self.set_up_test_model("test_rnflit", index_together=True) operation = migrations.RenameField("Pony", "pink", "blue") @@ -3062,6 +3070,7 @@ class OperationTests(OperationTestBase): with self.assertRaisesMessage(ValueError, msg): migrations.RenameIndex("Pony", new_name="new_idx_name") + @ignore_warnings(category=RemovedInDjango51Warning) def test_rename_index_unnamed_index(self): app_label = "test_rninui" project_state = self.set_up_test_model(app_label, index_together=True) @@ -3157,6 +3166,7 @@ class OperationTests(OperationTestBase): self.assertIsNot(old_model, new_model) self.assertEqual(new_model._meta.indexes[0].name, "new_pony_pink_idx") + @ignore_warnings(category=RemovedInDjango51Warning) def test_rename_index_state_forwards_unnamed_index(self): app_label = "test_rnidsfui" project_state = self.set_up_test_model(app_label, index_together=True) @@ -3291,6 +3301,7 @@ class OperationTests(OperationTestBase): # Ensure the index is still there self.assertIndexExists("test_alflin_pony", ["pink"]) + @ignore_warnings(category=RemovedInDjango51Warning) def test_alter_index_together(self): """ Tests the AlterIndexTogether operation. @@ -3343,6 +3354,7 @@ class OperationTests(OperationTestBase): definition[2], {"name": "Pony", "index_together": {("pink", "weight")}} ) + # RemovedInDjango51Warning. def test_alter_index_together_remove(self): operation = migrations.AlterIndexTogether("Pony", None) self.assertEqual( @@ -3350,6 +3362,7 @@ class OperationTests(OperationTestBase): ) @skipUnlessDBFeature("allows_multiple_constraints_on_same_fields") + @ignore_warnings(category=RemovedInDjango51Warning) def test_alter_index_together_remove_with_unique_together(self): app_label = "test_alintoremove_wunto" table_name = "%s_pony" % app_label diff --git a/tests/migrations/test_optimizer.py b/tests/migrations/test_optimizer.py index 6485009eb4..014f291db8 100644 --- a/tests/migrations/test_optimizer.py +++ b/tests/migrations/test_optimizer.py @@ -211,6 +211,7 @@ class OptimizerTests(SimpleTestCase): migrations.AlterUniqueTogether("Foo", [["a", "b"]]) ) + # RemovedInDjango51Warning. def test_create_alter_index_delete_model(self): self._test_create_alter_foo_delete_model( migrations.AlterIndexTogether("Foo", [["a", "b"]]) @@ -248,6 +249,7 @@ class OptimizerTests(SimpleTestCase): migrations.AlterUniqueTogether("Foo", [["a", "c"]]), ) + # RemovedInDjango51Warning. def test_alter_alter_index_model(self): self._test_alter_alter_model( migrations.AlterIndexTogether("Foo", [["a", "b"]]), @@ -1055,6 +1057,7 @@ class OptimizerTests(SimpleTestCase): migrations.AlterUniqueTogether("Foo", [["a", "b"]]) ) + # RemovedInDjango51Warning. def test_create_alter_index_field(self): self._test_create_alter_foo_field( migrations.AlterIndexTogether("Foo", [["a", "b"]]) diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 25f325246f..aad24eb6b0 100644 --- a/tests/migrations/test_state.py +++ b/tests/migrations/test_state.py @@ -13,8 +13,9 @@ from django.db.migrations.state import ( ProjectState, get_related_models_recursive, ) -from django.test import SimpleTestCase, override_settings +from django.test import SimpleTestCase, ignore_warnings, override_settings from django.test.utils import isolate_apps +from django.utils.deprecation import RemovedInDjango51Warning from .models import ( FoodManager, @@ -30,6 +31,9 @@ class StateTests(SimpleTestCase): Tests state construction, rendering and modification by operations. """ + # RemovedInDjango51Warning, when deprecation ends, only remove + # Meta.index_together from inline models. + @ignore_warnings(category=RemovedInDjango51Warning) def test_create(self): """ Tests making a ProjectState from an Apps @@ -46,7 +50,7 @@ class StateTests(SimpleTestCase): app_label = "migrations" apps = new_apps unique_together = ["name", "bio"] - index_together = ["bio", "age"] + index_together = ["bio", "age"] # RemovedInDjango51Warning. class AuthorProxy(Author): class Meta: @@ -140,7 +144,7 @@ class StateTests(SimpleTestCase): author_state.options, { "unique_together": {("name", "bio")}, - "index_together": {("bio", "age")}, + "index_together": {("bio", "age")}, # RemovedInDjango51Warning. "indexes": [], "constraints": [], }, diff --git a/tests/model_options/test_index_together_deprecation.py b/tests/model_options/test_index_together_deprecation.py new file mode 100644 index 0000000000..9b5362a924 --- /dev/null +++ b/tests/model_options/test_index_together_deprecation.py @@ -0,0 +1,20 @@ +from django.db import models +from django.test import TestCase +from django.utils.deprecation import RemovedInDjango51Warning + + +class IndexTogetherDeprecationTests(TestCase): + def test_warning(self): + msg = ( + "'index_together' is deprecated. Use 'Meta.indexes' in " + "'model_options.MyModel' instead." + ) + with self.assertRaisesMessage(RemovedInDjango51Warning, msg): + + class MyModel(models.Model): + field_1 = models.IntegerField() + field_2 = models.IntegerField() + + class Meta: + app_label = "model_options" + index_together = ["field_1", "field_2"] diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 7651b691b9..2868f59501 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -53,8 +53,14 @@ from django.db.models.fields.json import KeyTextTransform from django.db.models.functions import Abs, Cast, Collate, Lower, Random, Upper from django.db.models.indexes import IndexExpression from django.db.transaction import TransactionManagementError, atomic -from django.test import TransactionTestCase, skipIfDBFeature, skipUnlessDBFeature +from django.test import ( + TransactionTestCase, + ignore_warnings, + skipIfDBFeature, + skipUnlessDBFeature, +) from django.test.utils import CaptureQueriesContext, isolate_apps, register_lookup +from django.utils.deprecation import RemovedInDjango51Warning from .fields import CustomManyToManyField, InheritedManyToManyField, MediumBlobField from .models import ( @@ -2888,6 +2894,7 @@ class SchemaTests(TransactionTestCase): with self.assertRaises(DatabaseError): editor.add_constraint(Author, constraint) + @ignore_warnings(category=RemovedInDjango51Warning) def test_index_together(self): """ Tests removing and adding index_together constraints on a model. @@ -2931,6 +2938,7 @@ class SchemaTests(TransactionTestCase): False, ) + @ignore_warnings(category=RemovedInDjango51Warning) def test_index_together_with_fk(self): """ Tests removing and adding index_together constraints that include @@ -2949,6 +2957,7 @@ class SchemaTests(TransactionTestCase): with connection.schema_editor() as editor: editor.alter_index_together(Book, [["author", "title"]], []) + @ignore_warnings(category=RemovedInDjango51Warning) @isolate_apps("schema") def test_create_index_together(self): """ @@ -2978,6 +2987,7 @@ class SchemaTests(TransactionTestCase): ) @skipUnlessDBFeature("allows_multiple_constraints_on_same_fields") + @ignore_warnings(category=RemovedInDjango51Warning) @isolate_apps("schema") def test_remove_index_together_does_not_remove_meta_indexes(self): class AuthorWithIndexedNameAndBirthday(Model):