From 64b1ac7292c72d3551b2ad70b2a78c8fe4af3249 Mon Sep 17 00:00:00 2001 From: Georgi Yanchev Date: Mon, 20 Jan 2025 09:12:09 +0100 Subject: [PATCH] Fixed #24529 -- Allowed double squashing of migrations. Co-authored-by: Raphael Gaschignard --- .../management/commands/squashmigrations.py | 20 +-- django/db/migrations/loader.py | 32 ++++- docs/releases/6.0.txt | 3 +- docs/topics/migrations.txt | 13 +- tests/migrations/test_commands.py | 134 ++++++++++++++++++ .../test_migrations_squashed_loop/1_auto.py | 5 + .../test_migrations_squashed_loop/2_auto.py | 7 + .../2_squashed.py | 7 + .../test_migrations_squashed_loop/__init__.py | 0 .../0001_initial.py | 35 +++++ ...odel1_field_1_mymodel2_field_2_and_more.py | 23 +++ .../0003_alter_mymodel2_unique_together.py | 12 ++ ...odel1_field_1_mymodel1_field_3_and_more.py | 22 +++ .../__init__.py | 0 14 files changed, 291 insertions(+), 22 deletions(-) create mode 100644 tests/migrations/test_migrations_squashed_loop/1_auto.py create mode 100644 tests/migrations/test_migrations_squashed_loop/2_auto.py create mode 100644 tests/migrations/test_migrations_squashed_loop/2_squashed.py create mode 100644 tests/migrations/test_migrations_squashed_loop/__init__.py create mode 100644 tests/migrations/test_migrations_squashed_partially_applied/0001_initial.py create mode 100644 tests/migrations/test_migrations_squashed_partially_applied/0002_mymodel1_field_1_mymodel2_field_2_and_more.py create mode 100644 tests/migrations/test_migrations_squashed_partially_applied/0003_alter_mymodel2_unique_together.py create mode 100644 tests/migrations/test_migrations_squashed_partially_applied/0004_remove_mymodel1_field_1_mymodel1_field_3_and_more.py create mode 100644 tests/migrations/test_migrations_squashed_partially_applied/__init__.py diff --git a/django/core/management/commands/squashmigrations.py b/django/core/management/commands/squashmigrations.py index bb636116a5..9845b4d456 100644 --- a/django/core/management/commands/squashmigrations.py +++ b/django/core/management/commands/squashmigrations.py @@ -5,12 +5,11 @@ from django.apps import apps from django.conf import settings from django.core.management.base import BaseCommand, CommandError from django.core.management.utils import run_formatters -from django.db import DEFAULT_DB_ALIAS, connections, migrations +from django.db import migrations from django.db.migrations.loader import AmbiguityError, MigrationLoader from django.db.migrations.migration import SwappableTuple from django.db.migrations.optimizer import MigrationOptimizer from django.db.migrations.writer import MigrationWriter -from django.utils.version import get_docs_version class Command(BaseCommand): @@ -75,7 +74,7 @@ class Command(BaseCommand): raise CommandError(str(err)) # Load the current graph state, check the app and migration they asked # for exists. - loader = MigrationLoader(connections[DEFAULT_DB_ALIAS]) + loader = MigrationLoader(None) if app_label not in loader.migrated_apps: raise CommandError( "App '%s' does not have migrations (so squashmigrations on " @@ -141,12 +140,6 @@ class Command(BaseCommand): # as it may be 0002 depending on 0001 first_migration = True for smigration in migrations_to_squash: - if smigration.replaces: - raise CommandError( - "You cannot squash squashed migrations! Please transition it to a " - "normal migration first: https://docs.djangoproject.com/en/%s/" - "topics/migrations/#squashing-migrations" % get_docs_version() - ) operations.extend(smigration.operations) for dependency in smigration.dependencies: if isinstance(dependency, SwappableTuple): @@ -180,14 +173,7 @@ class Command(BaseCommand): % (len(operations), len(new_operations)) ) - # Work out the value of replaces (any squashed ones we're re-squashing) - # need to feed their replaces into ours - replaces = [] - for migration in migrations_to_squash: - if migration.replaces: - replaces.extend(migration.replaces) - else: - replaces.append((migration.app_label, migration.name)) + replaces = [(m.app_label, m.name) for m in migrations_to_squash] # Make a new migration with those operations subclass = type( diff --git a/django/db/migrations/loader.py b/django/db/migrations/loader.py index 3f5599bdf5..207be657b4 100644 --- a/django/db/migrations/loader.py +++ b/django/db/migrations/loader.py @@ -4,6 +4,7 @@ from importlib import import_module, reload from django.apps import apps from django.conf import settings +from django.core.management import CommandError from django.db.migrations.graph import MigrationGraph from django.db.migrations.recorder import MigrationRecorder @@ -219,11 +220,37 @@ class MigrationLoader: if child is not None: self.graph.add_dependency(migration, child, key, skip_validation=True) + def _resolve_replaced_migration_keys(self, migration): + resolved_keys = set() + for migration_key in set(migration.replaces): + migration_entry = self.disk_migrations.get(migration_key) + if migration_entry and migration_entry.replaces: + replace_keys = self._resolve_replaced_migration_keys(migration_entry) + resolved_keys.update(replace_keys) + else: + resolved_keys.add(migration_key) + return resolved_keys + def replace_migration(self, migration_key): + if completed_replacement := self.replacements_progress.get(migration_key, None): + return + elif completed_replacement is False: + # Called before but not finished the replacement, this means there + # is a circular dependency. + raise CommandError( + f"Cyclical squash replacement found, starting at {migration_key}" + ) + self.replacements_progress[migration_key] = False migration = self.replacements[migration_key] + # Process potential squashed migrations that the migration replaces. + for replace_migration_key in migration.replaces: + if replace_migration_key in self.replacements: + self.replace_migration(replace_migration_key) + + replaced_keys = self._resolve_replaced_migration_keys(migration) # Get applied status of each found replacement target. applied_statuses = [ - (target in self.applied_migrations) for target in migration.replaces + (target in self.applied_migrations) for target in replaced_keys ] # The replacing migration is only marked as applied if all of its # replacement targets are applied. @@ -241,6 +268,8 @@ class MigrationLoader: # dependencies to it (#25945). self.graph.remove_replacement_node(migration_key, migration.replaces) + self.replacements_progress[migration_key] = True + def build_graph(self): """ Build a migration dependency graph using both the disk and database. @@ -272,6 +301,7 @@ class MigrationLoader: self.add_external_dependencies(key, migration) # Carry out replacements where possible and if enabled. if self.replace_migrations: + self.replacements_progress = {} for migration_key in self.replacements.keys(): self.replace_migration(migration_key) # Ensure the graph is consistent. diff --git a/docs/releases/6.0.txt b/docs/releases/6.0.txt index 1496b1ef0e..9dabf7f666 100644 --- a/docs/releases/6.0.txt +++ b/docs/releases/6.0.txt @@ -169,7 +169,8 @@ Management Commands Migrations ~~~~~~~~~~ -* ... +* Squashed migrations can now themselves be squashed before being transitioned + to normal migrations. Models ~~~~~~ diff --git a/docs/topics/migrations.txt b/docs/topics/migrations.txt index f9748f0d9d..646b8d5116 100644 --- a/docs/topics/migrations.txt +++ b/docs/topics/migrations.txt @@ -733,7 +733,7 @@ migrations it replaces and distribute this change to all running instances of your application, making sure that they run ``migrate`` to store the change in their database. -You must then transition the squashed migration to a normal migration by: +You can then transition the squashed migration to a normal migration by: - Deleting all the migration files it replaces. - Updating all migrations that depend on the deleted migrations to depend on @@ -742,8 +742,11 @@ You must then transition the squashed migration to a normal migration by: squashed migration (this is how Django tells that it is a squashed migration). .. note:: - Once you've squashed a migration, you should not then re-squash that squashed - migration until you have fully transitioned it to a normal migration. + You can squash squashed migrations themselves without transitioning to + normal migrations, which might be useful for situations where every + environment has not yet run the original squashed migration set. But in + general it is better to transition squashed migrations to normal migrations + to be able to clean up older migration files. .. admonition:: Pruning references to deleted migrations @@ -751,6 +754,10 @@ You must then transition the squashed migration to a normal migration by: future, you should remove references to it from Django’s migrations table with the :option:`migrate --prune` option. +.. versionchanged:: 6.0 + + Support for squashing squashed migrations was added. + .. _migration-serializing: Serializing values diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index 5ff5cd4b26..dd54e4fe50 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -2,6 +2,7 @@ import datetime import importlib import io import os +import re import shutil import sys from pathlib import Path @@ -28,7 +29,9 @@ from django.db.backends.base.schema import BaseDatabaseSchemaEditor from django.db.backends.utils import truncate_name from django.db.migrations.autodetector import MigrationAutodetector from django.db.migrations.exceptions import InconsistentMigrationHistory +from django.db.migrations.loader import MigrationLoader from django.db.migrations.recorder import MigrationRecorder +from django.db.migrations.writer import MigrationWriter from django.test import TestCase, override_settings, skipUnlessDBFeature from django.test.utils import captured_stdout, extend_sys_path, isolate_apps from django.utils import timezone @@ -2939,6 +2942,137 @@ class SquashMigrationsTests(MigrationTestBase): " you can delete them.\n" % squashed_migration_file, ) + def test_squashmigrations_replacement_cycle(self): + out = io.StringIO() + with self.temporary_migration_module( + module="migrations.test_migrations_squashed_loop" + ): + # Hits a squash replacement cycle check error, but the actual failure is + # dependent on the order in which the files are read on disk. + with self.assertRaisesRegex( + CommandError, + r"Cyclical squash replacement found, starting at" + r" \('migrations', '2_(squashed|auto)'\)", + ): + call_command( + "migrate", "migrations", "--plan", interactive=False, stdout=out + ) + + def test_squashmigrations_squashes_already_squashed(self): + out = io.StringIO() + + with self.temporary_migration_module( + module="migrations.test_migrations_squashed_complex" + ): + call_command( + "squashmigrations", + "migrations", + "3_squashed_5", + "--squashed-name", + "double_squash", + stdout=out, + interactive=False, + ) + + loader = MigrationLoader(connection) + migration = loader.disk_migrations[("migrations", "0001_double_squash")] + # Confirm the replaces mechanism holds the squashed migration + # (and not what it squashes, as the squash operations are what + # end up being used). + self.assertEqual( + migration.replaces, + [ + ("migrations", "1_auto"), + ("migrations", "2_auto"), + ("migrations", "3_squashed_5"), + ], + ) + + out = io.StringIO() + call_command( + "migrate", "migrations", "--plan", interactive=False, stdout=out + ) + + migration_plan = re.findall("migrations.(.+)\n", out.getvalue()) + self.assertEqual(migration_plan, ["0001_double_squash", "6_auto", "7_auto"]) + + def test_squash_partially_applied(self): + """ + Replacement migrations are partially applied. Then we squash again and + verify that only unapplied migrations will be applied by "migrate". + """ + out = io.StringIO() + + with self.temporary_migration_module( + module="migrations.test_migrations_squashed_partially_applied" + ): + # Apply first 2 migrations. + call_command("migrate", "migrations", "0002", interactive=False, stdout=out) + + # Squash the 2 migrations, that we just applied + 1 more. + call_command( + "squashmigrations", + "migrations", + "0001", + "0003", + "--squashed-name", + "squashed_0001_0003", + stdout=out, + interactive=False, + ) + + # Update the 4th migration to depend on the squash(replacement) migration. + loader = MigrationLoader(connection) + migration = loader.disk_migrations[ + ("migrations", "0004_remove_mymodel1_field_1_mymodel1_field_3_and_more") + ] + migration.dependencies = [("migrations", "0001_squashed_0001_0003")] + writer = MigrationWriter(migration) + with open(writer.path, "w", encoding="utf-8") as fh: + fh.write(writer.as_string()) + + # Squash the squash(replacement) migration with the 4th migration. + call_command( + "squashmigrations", + "migrations", + "0001_squashed_0001_0003", + "0004", + "--squashed-name", + "squashed_0001_0004", + stdout=out, + interactive=False, + ) + + loader = MigrationLoader(connection) + migration = loader.disk_migrations[ + ("migrations", "0001_squashed_0001_0004") + ] + self.assertEqual( + migration.replaces, + [ + ("migrations", "0001_squashed_0001_0003"), + ( + "migrations", + "0004_remove_mymodel1_field_1_mymodel1_field_3_and_more", + ), + ], + ) + + # Verify that only unapplied migrations will be applied. + out = io.StringIO() + call_command( + "migrate", "migrations", "--plan", interactive=False, stdout=out + ) + + migration_plan = re.findall("migrations.(.+)\n", out.getvalue()) + self.assertEqual( + migration_plan, + [ + "0003_alter_mymodel2_unique_together", + "0004_remove_mymodel1_field_1_mymodel1_field_3_and_more", + ], + ) + def test_squashmigrations_initial_attribute(self): with self.temporary_migration_module( module="migrations.test_migrations" diff --git a/tests/migrations/test_migrations_squashed_loop/1_auto.py b/tests/migrations/test_migrations_squashed_loop/1_auto.py new file mode 100644 index 0000000000..2e5fb93b75 --- /dev/null +++ b/tests/migrations/test_migrations_squashed_loop/1_auto.py @@ -0,0 +1,5 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + operations = [migrations.RunPython(migrations.RunPython.noop)] diff --git a/tests/migrations/test_migrations_squashed_loop/2_auto.py b/tests/migrations/test_migrations_squashed_loop/2_auto.py new file mode 100644 index 0000000000..e6ed47e97d --- /dev/null +++ b/tests/migrations/test_migrations_squashed_loop/2_auto.py @@ -0,0 +1,7 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + replaces = [("migrations", "2_squashed")] + dependencies = [("migrations", "1_auto")] + operations = [migrations.RunPython(migrations.RunPython.noop)] diff --git a/tests/migrations/test_migrations_squashed_loop/2_squashed.py b/tests/migrations/test_migrations_squashed_loop/2_squashed.py new file mode 100644 index 0000000000..d26d8daf27 --- /dev/null +++ b/tests/migrations/test_migrations_squashed_loop/2_squashed.py @@ -0,0 +1,7 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + replaces = [("migrations", "2_auto")] + dependencies = [("migrations", "1_auto")] + operations = [migrations.RunPython(migrations.RunPython.noop)] diff --git a/tests/migrations/test_migrations_squashed_loop/__init__.py b/tests/migrations/test_migrations_squashed_loop/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/test_migrations_squashed_partially_applied/0001_initial.py b/tests/migrations/test_migrations_squashed_partially_applied/0001_initial.py new file mode 100644 index 0000000000..ed6b825ab0 --- /dev/null +++ b/tests/migrations/test_migrations_squashed_partially_applied/0001_initial.py @@ -0,0 +1,35 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + operations = [ + migrations.CreateModel( + name="MyModel1", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ], + ), + migrations.CreateModel( + name="MyModel2", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("field_1", models.IntegerField()), + ], + ), + ] diff --git a/tests/migrations/test_migrations_squashed_partially_applied/0002_mymodel1_field_1_mymodel2_field_2_and_more.py b/tests/migrations/test_migrations_squashed_partially_applied/0002_mymodel1_field_1_mymodel2_field_2_and_more.py new file mode 100644 index 0000000000..a102e65f68 --- /dev/null +++ b/tests/migrations/test_migrations_squashed_partially_applied/0002_mymodel1_field_1_mymodel2_field_2_and_more.py @@ -0,0 +1,23 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("migrations", "0001_initial")] + + operations = [ + migrations.AddField( + model_name="mymodel1", + name="field_1", + field=models.IntegerField(null=True), + ), + migrations.AddField( + model_name="mymodel2", + name="field_2", + field=models.IntegerField(null=True), + ), + migrations.AlterField( + model_name="mymodel2", + name="field_1", + field=models.IntegerField(null=True), + ), + ] diff --git a/tests/migrations/test_migrations_squashed_partially_applied/0003_alter_mymodel2_unique_together.py b/tests/migrations/test_migrations_squashed_partially_applied/0003_alter_mymodel2_unique_together.py new file mode 100644 index 0000000000..8fb7a129e3 --- /dev/null +++ b/tests/migrations/test_migrations_squashed_partially_applied/0003_alter_mymodel2_unique_together.py @@ -0,0 +1,12 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [("migrations", "0002_mymodel1_field_1_mymodel2_field_2_and_more")] + + operations = [ + migrations.AlterUniqueTogether( + name="mymodel2", + unique_together={("field_1", "field_2")}, + ), + ] diff --git a/tests/migrations/test_migrations_squashed_partially_applied/0004_remove_mymodel1_field_1_mymodel1_field_3_and_more.py b/tests/migrations/test_migrations_squashed_partially_applied/0004_remove_mymodel1_field_1_mymodel1_field_3_and_more.py new file mode 100644 index 0000000000..96a34a4a11 --- /dev/null +++ b/tests/migrations/test_migrations_squashed_partially_applied/0004_remove_mymodel1_field_1_mymodel1_field_3_and_more.py @@ -0,0 +1,22 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("migrations", "0003_alter_mymodel2_unique_together")] + + operations = [ + migrations.RemoveField( + model_name="mymodel1", + name="field_1", + ), + migrations.AddField( + model_name="mymodel1", + name="field_3", + field=models.IntegerField(null=True), + ), + migrations.AddField( + model_name="mymodel1", + name="field_4", + field=models.IntegerField(null=True), + ), + ] diff --git a/tests/migrations/test_migrations_squashed_partially_applied/__init__.py b/tests/migrations/test_migrations_squashed_partially_applied/__init__.py new file mode 100644 index 0000000000..e69de29bb2