Browse Source

Fixed #27236 -- Deprecated Meta.index_together in favor of Meta.indexes.

This also deprecates AlterIndexTogether migration operation.
David Wobrock 2 years ago
parent
commit
a6385b382e

+ 8 - 0
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)
 

+ 8 - 0
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):

+ 5 - 0
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

+ 6 - 0
docs/ref/checks.txt

@@ -397,6 +397,12 @@ Models
 * **models.W045**: Check constraint ``<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
 --------
 

+ 7 - 0
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``
 ---------------------------
 

+ 4 - 6
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``
 ---------------
 

+ 32 - 0
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 <django.db.models.Options.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
   <https://docs.python.org/3/library/secrets.html#recipes-and-best-practices>`_
   for using Python's :py:mod:`secrets` module to generate passwords.
+
+* The ``AlterIndexTogether`` migration operation is deprecated.

+ 0 - 0
tests/check_framework/migrations_test_apps/__init__.py


+ 0 - 0
tests/check_framework/migrations_test_apps/index_together_app/__init__.py


+ 5 - 0
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"

+ 16 - 0
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"),)),
+    ]

+ 0 - 0
tests/check_framework/migrations_test_apps/index_together_app/migrations/__init__.py


+ 21 - 0
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):

+ 3 - 0
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):

+ 3 - 1
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):

+ 3 - 1
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",

+ 2 - 1
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 [],
         }

+ 14 - 1
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

+ 3 - 0
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"]])

+ 7 - 3
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": [],
             },

+ 20 - 0
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"]

+ 11 - 1
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):