From fa58450a9ab8a1bdd2a5090b51b00078fd85ffa6 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Sat, 23 Nov 2019 11:08:45 +0000 Subject: [PATCH] Fixed #31468 -- Allowed specifying migration filename in Operation. This adds also suggested filename for many built-in operations. --- django/contrib/postgres/operations.py | 4 ++ django/db/migrations/autodetector.py | 19 ++++----- django/db/migrations/operations/base.py | 8 ++++ django/db/migrations/operations/fields.py | 20 ++++++++++ django/db/migrations/operations/models.py | 48 +++++++++++++++++++++++ docs/ref/migration-operations.txt | 14 +++++++ docs/releases/3.2.txt | 4 +- tests/migrations/test_autodetector.py | 5 +++ tests/migrations/test_operations.py | 38 ++++++++++++++++++ tests/postgres_tests/test_operations.py | 2 + 10 files changed, 149 insertions(+), 13 deletions(-) diff --git a/django/contrib/postgres/operations.py b/django/contrib/postgres/operations.py index 740911d0595..b25438b8267 100644 --- a/django/contrib/postgres/operations.py +++ b/django/contrib/postgres/operations.py @@ -55,6 +55,10 @@ class CreateExtension(Operation): def describe(self): return "Creates extension %s" % self.name + @property + def migration_name_fragment(self): + return 'create_extension_%s' % self.name + class BloomExtension(CreateExtension): diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 85c30138975..a6edc2cd6ef 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -1313,19 +1313,14 @@ class MigrationAutodetector: represent. Names are not guaranteed to be unique, but put some effort into the fallback name to avoid VCS conflicts if possible. """ + name = None if len(ops) == 1: - if isinstance(ops[0], operations.CreateModel): - return ops[0].name_lower - elif isinstance(ops[0], operations.DeleteModel): - return "delete_%s" % ops[0].name_lower - elif isinstance(ops[0], operations.AddField): - return "%s_%s" % (ops[0].model_name_lower, ops[0].name_lower) - elif isinstance(ops[0], operations.RemoveField): - return "remove_%s_%s" % (ops[0].model_name_lower, ops[0].name_lower) - elif ops: - if all(isinstance(o, operations.CreateModel) for o in ops): - return "_".join(sorted(o.name_lower for o in ops)) - return "auto_%s" % get_migration_name_timestamp() + name = ops[0].migration_name_fragment + elif len(ops) > 1 and all(isinstance(o, operations.CreateModel) for o in ops): + name = '_'.join(sorted(o.migration_name_fragment for o in ops)) + if name is None: + name = 'auto_%s' % get_migration_name_timestamp() + return name @classmethod def parse_number(cls, name): diff --git a/django/db/migrations/operations/base.py b/django/db/migrations/operations/base.py index 35fdd723106..18935520f88 100644 --- a/django/db/migrations/operations/base.py +++ b/django/db/migrations/operations/base.py @@ -79,6 +79,14 @@ class Operation: """ return "%s: %s" % (self.__class__.__name__, self._constructor_args) + @property + def migration_name_fragment(self): + """ + A filename part suitable for automatically naming a migration + containing this operation, or None if not applicable. + """ + return None + def references_model(self, name, app_label): """ Return True if there is a chance this operation references the given diff --git a/django/db/migrations/operations/fields.py b/django/db/migrations/operations/fields.py index e4f78e365eb..8e494fd595c 100644 --- a/django/db/migrations/operations/fields.py +++ b/django/db/migrations/operations/fields.py @@ -116,6 +116,10 @@ class AddField(FieldOperation): def describe(self): return "Add field %s to %s" % (self.name, self.model_name) + @property + def migration_name_fragment(self): + return '%s_%s' % (self.model_name_lower, self.name_lower) + def reduce(self, operation, app_label): if isinstance(operation, FieldOperation) and self.is_same_field_operation(operation): if isinstance(operation, AlterField): @@ -174,6 +178,10 @@ class RemoveField(FieldOperation): def describe(self): return "Remove field %s from %s" % (self.name, self.model_name) + @property + def migration_name_fragment(self): + return 'remove_%s_%s' % (self.model_name_lower, self.name_lower) + def reduce(self, operation, app_label): from .models import DeleteModel if isinstance(operation, DeleteModel) and operation.name_lower == self.model_name_lower: @@ -243,6 +251,10 @@ class AlterField(FieldOperation): def describe(self): return "Alter field %s on %s" % (self.name, self.model_name) + @property + def migration_name_fragment(self): + return 'alter_%s_%s' % (self.model_name_lower, self.name_lower) + def reduce(self, operation, app_label): if isinstance(operation, RemoveField) and self.is_same_field_operation(operation): return [operation] @@ -354,6 +366,14 @@ class RenameField(FieldOperation): def describe(self): return "Rename field %s on %s to %s" % (self.old_name, self.model_name, self.new_name) + @property + def migration_name_fragment(self): + return 'rename_%s_%s_%s' % ( + self.old_name_lower, + self.model_name_lower, + self.new_name_lower, + ) + def references_field(self, model_name, name, app_label): return self.references_model(model_name, app_label) and ( name.lower() == self.old_name_lower or diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 8c06c4d157b..dddf7c679c1 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -99,6 +99,10 @@ class CreateModel(ModelOperation): def describe(self): return "Create %smodel %s" % ("proxy " if self.options.get("proxy", False) else "", self.name) + @property + def migration_name_fragment(self): + return self.name_lower + def references_model(self, name, app_label): name_lower = name.lower() if name_lower == self.name_lower: @@ -273,6 +277,10 @@ class DeleteModel(ModelOperation): def describe(self): return "Delete model %s" % self.name + @property + def migration_name_fragment(self): + return 'delete_%s' % self.name_lower + class RenameModel(ModelOperation): """Rename a model.""" @@ -397,6 +405,10 @@ class RenameModel(ModelOperation): def describe(self): return "Rename model %s to %s" % (self.old_name, self.new_name) + @property + def migration_name_fragment(self): + return 'rename_%s_%s' % (self.old_name_lower, self.new_name_lower) + def reduce(self, operation, app_label): if (isinstance(operation, RenameModel) and self.new_name_lower == operation.old_name_lower): @@ -470,6 +482,10 @@ class AlterModelTable(ModelOptionOperation): self.table if self.table is not None else "(default)" ) + @property + def migration_name_fragment(self): + return 'alter_%s_table' % self.name_lower + class AlterTogetherOptionOperation(ModelOptionOperation): option_name = None @@ -526,6 +542,10 @@ class AlterTogetherOptionOperation(ModelOptionOperation): def describe(self): return "Alter %s for %s (%s constraint(s))" % (self.option_name, self.name, len(self.option_value or '')) + @property + def migration_name_fragment(self): + return 'alter_%s_%s' % (self.name_lower, self.option_name) + class AlterUniqueTogether(AlterTogetherOptionOperation): """ @@ -607,6 +627,10 @@ class AlterOrderWithRespectTo(ModelOptionOperation): def describe(self): return "Set order_with_respect_to on %s to %s" % (self.name, self.order_with_respect_to) + @property + def migration_name_fragment(self): + return 'alter_%s_order_with_respect_to' % self.name_lower + class AlterModelOptions(ModelOptionOperation): """ @@ -662,6 +686,10 @@ class AlterModelOptions(ModelOptionOperation): def describe(self): return "Change Meta options on %s" % self.name + @property + def migration_name_fragment(self): + return 'alter_%s_options' % self.name_lower + class AlterModelManagers(ModelOptionOperation): """Alter the model's managers.""" @@ -693,6 +721,10 @@ class AlterModelManagers(ModelOptionOperation): def describe(self): return "Change managers on %s" % self.name + @property + def migration_name_fragment(self): + return 'alter_%s_managers' % self.name_lower + class IndexOperation(Operation): option_name = 'indexes' @@ -747,6 +779,10 @@ class AddIndex(IndexOperation): self.model_name, ) + @property + def migration_name_fragment(self): + return '%s_%s' % (self.model_name_lower, self.index.name.lower()) + class RemoveIndex(IndexOperation): """Remove an index from a model.""" @@ -789,6 +825,10 @@ class RemoveIndex(IndexOperation): def describe(self): return 'Remove index %s from %s' % (self.name, self.model_name) + @property + def migration_name_fragment(self): + return 'remove_%s_%s' % (self.model_name_lower, self.name.lower()) + class AddConstraint(IndexOperation): option_name = 'constraints' @@ -821,6 +861,10 @@ class AddConstraint(IndexOperation): def describe(self): return 'Create constraint %s on model %s' % (self.constraint.name, self.model_name) + @property + def migration_name_fragment(self): + return '%s_%s' % (self.model_name_lower, self.constraint.name.lower()) + class RemoveConstraint(IndexOperation): option_name = 'constraints' @@ -857,3 +901,7 @@ class RemoveConstraint(IndexOperation): def describe(self): return 'Remove constraint %s from model %s' % (self.name, self.model_name) + + @property + def migration_name_fragment(self): + return 'remove_%s_%s' % (self.model_name_lower, self.name.lower()) diff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt index a6b42bdc997..d1620cce8e6 100644 --- a/docs/ref/migration-operations.txt +++ b/docs/ref/migration-operations.txt @@ -484,6 +484,16 @@ structure of an ``Operation`` looks like this:: # This is used to describe what the operation does in console output. return "Custom Operation" + @property + def migration_name_fragment(self): + # Optional. A filename part suitable for automatically naming a + # migration containing this operation, or None if not applicable. + return "custom_operation_%s_%s" % (self.arg1, self.arg2) + +.. versionadded:: 3.2 + + The ``migration_name_fragment`` property was added. + You can take this template and work from it, though we suggest looking at the built-in Django operations in ``django.db.migrations.operations`` - they cover a lot of the example usage of semi-internal aspects of the migration framework @@ -553,3 +563,7 @@ state changes, all it does is run one command:: def describe(self): return "Creates extension %s" % self.name + + @property + def migration_name_fragment(self): + return "create_extension_%s" % self.name diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index 1ba636c4a6f..a869eb49cf7 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -171,7 +171,9 @@ Management Commands Migrations ~~~~~~~~~~ -* ... +* The new ``Operation.migration_name_fragment`` property allows providing a + filename fragment that will be used to name a migration containing only that + operation. Models ~~~~~~ diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 52d8e305ce6..ab52b8116d6 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -2495,6 +2495,11 @@ class AutodetectorSuggestNameTests(SimpleTestCase): ] self.assertEqual(MigrationAutodetector.suggest_name(ops), 'animal_person') + def test_none_name(self): + ops = [migrations.RunSQL('SELECT 1 FROM person;')] + suggest_name = MigrationAutodetector.suggest_name(ops) + self.assertIs(suggest_name.startswith('auto_'), True) + def test_auto(self): suggest_name = MigrationAutodetector.suggest_name([]) self.assertIs(suggest_name.startswith('auto_'), True) diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index e985535679e..855a0520587 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -36,6 +36,7 @@ class OperationTests(OperationTestBase): ], ) self.assertEqual(operation.describe(), "Create model Pony") + self.assertEqual(operation.migration_name_fragment, 'pony') # Test the state alteration project_state = ProjectState() new_state = project_state.clone() @@ -486,6 +487,7 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.DeleteModel("Pony") self.assertEqual(operation.describe(), "Delete model Pony") + self.assertEqual(operation.migration_name_fragment, 'delete_pony') new_state = project_state.clone() operation.state_forwards("test_dlmo", new_state) self.assertNotIn(("test_dlmo", "pony"), new_state.models) @@ -559,6 +561,7 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.RenameModel("Pony", "Horse") self.assertEqual(operation.describe(), "Rename model Pony to Horse") + self.assertEqual(operation.migration_name_fragment, 'rename_pony_horse') # Test initial state and database self.assertIn(("test_rnmo", "pony"), project_state.models) self.assertNotIn(("test_rnmo", "horse"), project_state.models) @@ -855,6 +858,7 @@ class OperationTests(OperationTestBase): models.FloatField(null=True, default=5), ) self.assertEqual(operation.describe(), "Add field height to Pony") + self.assertEqual(operation.migration_name_fragment, 'pony_height') project_state, new_state = self.make_test_state("test_adfl", operation) self.assertEqual(len(new_state.models["test_adfl", "pony"].fields), 4) field = new_state.models['test_adfl', 'pony'].fields['height'] @@ -1155,6 +1159,7 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.RemoveField("Pony", "pink") self.assertEqual(operation.describe(), "Remove field pink from Pony") + self.assertEqual(operation.migration_name_fragment, 'remove_pony_pink') new_state = project_state.clone() operation.state_forwards("test_rmfl", new_state) self.assertEqual(len(new_state.models["test_rmfl", "pony"].fields), 2) @@ -1198,6 +1203,7 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.AlterModelTable("Pony", "test_almota_pony_2") self.assertEqual(operation.describe(), "Rename table for Pony to test_almota_pony_2") + self.assertEqual(operation.migration_name_fragment, 'alter_pony_table') new_state = project_state.clone() operation.state_forwards("test_almota", new_state) self.assertEqual(new_state.models["test_almota", "pony"].options["db_table"], "test_almota_pony_2") @@ -1286,6 +1292,7 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.AlterField("Pony", "pink", models.IntegerField(null=True)) self.assertEqual(operation.describe(), "Alter field pink on Pony") + self.assertEqual(operation.migration_name_fragment, 'alter_pony_pink') new_state = project_state.clone() operation.state_forwards("test_alfl", new_state) self.assertIs(project_state.models['test_alfl', 'pony'].fields['pink'].null, False) @@ -1543,6 +1550,7 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.RenameField("Pony", "pink", "blue") self.assertEqual(operation.describe(), "Rename field pink on Pony to blue") + self.assertEqual(operation.migration_name_fragment, 'rename_pink_pony_blue') new_state = project_state.clone() operation.state_forwards("test_rnfl", new_state) self.assertIn("blue", new_state.models["test_rnfl", "pony"].fields) @@ -1624,6 +1632,10 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.AlterUniqueTogether("Pony", [("pink", "weight")]) self.assertEqual(operation.describe(), "Alter unique_together for Pony (1 constraint(s))") + self.assertEqual( + operation.migration_name_fragment, + 'alter_pony_unique_together', + ) new_state = project_state.clone() operation.state_forwards("test_alunto", new_state) self.assertEqual(len(project_state.models["test_alunto", "pony"].options.get("unique_together", set())), 0) @@ -1675,6 +1687,10 @@ class OperationTests(OperationTestBase): index = models.Index(fields=["pink"], name="test_adin_pony_pink_idx") operation = migrations.AddIndex("Pony", index) self.assertEqual(operation.describe(), "Create index test_adin_pony_pink_idx on field(s) pink of model Pony") + self.assertEqual( + operation.migration_name_fragment, + 'pony_test_adin_pony_pink_idx', + ) new_state = project_state.clone() operation.state_forwards("test_adin", new_state) # Test the database alteration @@ -1702,6 +1718,10 @@ class OperationTests(OperationTestBase): self.assertIndexExists("test_rmin_pony", ["pink", "weight"]) operation = migrations.RemoveIndex("Pony", "pony_test_idx") self.assertEqual(operation.describe(), "Remove index pony_test_idx from Pony") + self.assertEqual( + operation.migration_name_fragment, + 'remove_pony_pony_test_idx', + ) new_state = project_state.clone() operation.state_forwards("test_rmin", new_state) # Test the state alteration @@ -1789,6 +1809,10 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.AlterIndexTogether("Pony", [("pink", "weight")]) self.assertEqual(operation.describe(), "Alter index_together for Pony (1 constraint(s))") + self.assertEqual( + operation.migration_name_fragment, + 'alter_pony_index_together', + ) new_state = project_state.clone() operation.state_forwards("test_alinto", new_state) self.assertEqual(len(project_state.models["test_alinto", "pony"].options.get("index_together", set())), 0) @@ -1845,6 +1869,10 @@ class OperationTests(OperationTestBase): self.assertEqual( gt_operation.describe(), "Create constraint test_add_constraint_pony_pink_gt_2 on model Pony" ) + self.assertEqual( + gt_operation.migration_name_fragment, + 'pony_test_add_constraint_pony_pink_gt_2', + ) # Test the state alteration new_state = project_state.clone() gt_operation.state_forwards("test_addconstraint", new_state) @@ -1982,6 +2010,10 @@ class OperationTests(OperationTestBase): self.assertEqual( gt_operation.describe(), "Remove constraint test_remove_constraint_pony_pink_gt_2 from model Pony" ) + self.assertEqual( + gt_operation.migration_name_fragment, + 'remove_pony_test_remove_constraint_pony_pink_gt_2', + ) # Test state alteration new_state = project_state.clone() gt_operation.state_forwards("test_removeconstraint", new_state) @@ -2212,6 +2244,7 @@ class OperationTests(OperationTestBase): # Test the state alteration (no DB alteration to test) operation = migrations.AlterModelOptions("Pony", {"permissions": [("can_groom", "Can groom")]}) self.assertEqual(operation.describe(), "Change Meta options on Pony") + self.assertEqual(operation.migration_name_fragment, 'alter_pony_options') new_state = project_state.clone() operation.state_forwards("test_almoop", new_state) self.assertEqual(len(project_state.models["test_almoop", "pony"].options.get("permissions", [])), 0) @@ -2249,6 +2282,10 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.AlterOrderWithRespectTo("Rider", "pony") self.assertEqual(operation.describe(), "Set order_with_respect_to on Rider to pony") + self.assertEqual( + operation.migration_name_fragment, + 'alter_rider_order_with_respect_to', + ) new_state = project_state.clone() operation.state_forwards("test_alorwrtto", new_state) self.assertIsNone( @@ -2298,6 +2335,7 @@ class OperationTests(OperationTestBase): ] ) self.assertEqual(operation.describe(), "Change managers on Pony") + self.assertEqual(operation.migration_name_fragment, 'alter_pony_managers') managers = project_state.models["test_almoma", "pony"].managers self.assertEqual(managers, []) diff --git a/tests/postgres_tests/test_operations.py b/tests/postgres_tests/test_operations.py index 8cc9a2b66ed..a10f5da4402 100644 --- a/tests/postgres_tests/test_operations.py +++ b/tests/postgres_tests/test_operations.py @@ -175,6 +175,7 @@ class CreateExtensionTests(PostgreSQLTestCase): def test_allow_migrate(self): operation = CreateExtension('tablefunc') + self.assertEqual(operation.migration_name_fragment, 'create_extension_tablefunc') project_state = ProjectState() new_state = project_state.clone() # Create an extension. @@ -192,6 +193,7 @@ class CreateExtensionTests(PostgreSQLTestCase): def test_create_existing_extension(self): operation = BloomExtension() + self.assertEqual(operation.migration_name_fragment, 'create_extension_bloom') project_state = ProjectState() new_state = project_state.clone() # Don't create an existing extension.