From 95bda03f2da15172cf342f13ba8a77c007b63fbb Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 29 Oct 2018 05:33:41 -0400 Subject: [PATCH] Fixed #29868 -- Retained database constraints on SQLite table rebuilds. Refs #11964. Thanks Scott Stevens for testing this upcoming feature and the report. --- django/db/backends/sqlite3/schema.py | 14 ++---- django/db/migrations/operations/models.py | 4 +- tests/migrations/test_operations.py | 59 +++++++++++++++++------ 3 files changed, 50 insertions(+), 27 deletions(-) diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index 99382d35ce5..7aa1f28f536 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/django/db/backends/sqlite3/schema.py @@ -126,8 +126,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): else: super().alter_field(model, old_field, new_field, strict=strict) - def _remake_table(self, model, create_field=None, delete_field=None, alter_field=None, - add_constraint=None, remove_constraint=None): + def _remake_table(self, model, create_field=None, delete_field=None, alter_field=None): """ Shortcut to transform a model from old_model into new_model @@ -224,13 +223,6 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): ] constraints = list(model._meta.constraints) - if add_constraint: - constraints.append(add_constraint) - if remove_constraint: - constraints = [ - constraint for constraint in constraints - if remove_constraint.name != constraint.name - ] # Construct a new model for the new state meta_contents = { @@ -383,7 +375,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): self.delete_model(old_field.remote_field.through) def add_constraint(self, model, constraint): - self._remake_table(model, add_constraint=constraint) + self._remake_table(model) def remove_constraint(self, model, constraint): - self._remake_table(model, remove_constraint=constraint) + self._remake_table(model) diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 9b13bb9a3c8..3241f53d3cb 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -819,6 +819,7 @@ class AddConstraint(IndexOperation): def state_forwards(self, app_label, state): model_state = state.models[app_label, self.model_name_lower] model_state.options[self.option_name] = [*model_state.options[self.option_name], self.constraint] + state.reload_model(app_label, self.model_name_lower, delay=True) def database_forwards(self, app_label, schema_editor, from_state, to_state): model = to_state.apps.get_model(app_label, self.model_name) @@ -851,9 +852,10 @@ class RemoveConstraint(IndexOperation): model_state = state.models[app_label, self.model_name_lower] constraints = model_state.options[self.option_name] model_state.options[self.option_name] = [c for c in constraints if c.name != self.name] + state.reload_model(app_label, self.model_name_lower, delay=True) def database_forwards(self, app_label, schema_editor, from_state, to_state): - model = from_state.apps.get_model(app_label, self.model_name) + model = to_state.apps.get_model(app_label, self.model_name) if self.allow_migrate_model(schema_editor.connection.alias, model): from_model_state = from_state.models[app_label, self.model_name_lower] constraint = from_model_state.get_constraint_by_name(self.name) diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 5447912c6bd..3e54dffa7dc 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -54,7 +54,7 @@ class OperationTestBase(MigrationTestBase): def set_up_test_model( self, app_label, second_model=False, third_model=False, index=False, multicol_index=False, related_model=False, mti_model=False, proxy_model=False, manager_model=False, - unique_together=False, options=False, db_table=None, index_together=False, check_constraint=False): + unique_together=False, options=False, db_table=None, index_together=False, constraints=None): """ Creates a test model state and database table. """ @@ -107,11 +107,12 @@ class OperationTestBase(MigrationTestBase): "Pony", models.Index(fields=["pink", "weight"], name="pony_test_idx") )) - if check_constraint: - operations.append(migrations.AddConstraint( - "Pony", - models.CheckConstraint(check=models.Q(pink__gt=2), name="pony_test_constraint") - )) + if constraints: + for constraint in constraints: + operations.append(migrations.AddConstraint( + "Pony", + constraint, + )) if second_model: operations.append(migrations.CreateModel( "Stable", @@ -1788,11 +1789,24 @@ class OperationTests(OperationTestBase): gt_operation.state_forwards("test_addconstraint", new_state) self.assertEqual(len(new_state.models["test_addconstraint", "pony"].options["constraints"]), 1) Pony = new_state.apps.get_model("test_addconstraint", "Pony") + self.assertEqual(len(Pony._meta.constraints), 1) # Test the database alteration with connection.schema_editor() as editor: gt_operation.database_forwards("test_addconstraint", editor, project_state, new_state) with self.assertRaises(IntegrityError), transaction.atomic(): Pony.objects.create(pink=1, weight=1.0) + # Add another one. + lt_check = models.Q(pink__lt=100) + lt_constraint = models.CheckConstraint(check=lt_check, name="test_constraint_pony_pink_lt_100") + lt_operation = migrations.AddConstraint("Pony", lt_constraint) + lt_operation.state_forwards("test_addconstraint", new_state) + self.assertEqual(len(new_state.models["test_addconstraint", "pony"].options["constraints"]), 2) + Pony = new_state.apps.get_model("test_addconstraint", "Pony") + self.assertEqual(len(Pony._meta.constraints), 2) + with connection.schema_editor() as editor: + lt_operation.database_forwards("test_addconstraint", editor, project_state, new_state) + with self.assertRaises(IntegrityError), transaction.atomic(): + Pony.objects.create(pink=100, weight=1.0) # Test reversal with connection.schema_editor() as editor: gt_operation.database_backwards("test_addconstraint", editor, new_state, project_state) @@ -1805,28 +1819,43 @@ class OperationTests(OperationTestBase): @skipUnlessDBFeature('supports_table_check_constraints') def test_remove_constraint(self): - project_state = self.set_up_test_model("test_removeconstraint", check_constraint=True) - operation = migrations.RemoveConstraint("Pony", "pony_test_constraint") - self.assertEqual(operation.describe(), "Remove constraint pony_test_constraint from model Pony") + project_state = self.set_up_test_model("test_removeconstraint", constraints=[ + models.CheckConstraint(check=models.Q(pink__gt=2), name="test_constraint_pony_pink_gt_2"), + models.CheckConstraint(check=models.Q(pink__lt=100), name="test_constraint_pony_pink_lt_100"), + ]) + gt_operation = migrations.RemoveConstraint("Pony", "test_constraint_pony_pink_gt_2") + self.assertEqual(gt_operation.describe(), "Remove constraint test_constraint_pony_pink_gt_2 from model Pony") # Test state alteration new_state = project_state.clone() - operation.state_forwards("test_removeconstraint", new_state) - self.assertEqual(len(new_state.models["test_removeconstraint", "pony"].options['constraints']), 0) + gt_operation.state_forwards("test_removeconstraint", new_state) + self.assertEqual(len(new_state.models["test_removeconstraint", "pony"].options['constraints']), 1) Pony = new_state.apps.get_model("test_removeconstraint", "Pony") + self.assertEqual(len(Pony._meta.constraints), 1) # Test database alteration with connection.schema_editor() as editor: - operation.database_forwards("test_removeconstraint", editor, project_state, new_state) + gt_operation.database_forwards("test_removeconstraint", editor, project_state, new_state) Pony.objects.create(pink=1, weight=1.0).delete() + with self.assertRaises(IntegrityError), transaction.atomic(): + Pony.objects.create(pink=100, weight=1.0) + # Remove the other one. + lt_operation = migrations.RemoveConstraint("Pony", "test_constraint_pony_pink_lt_100") + lt_operation.state_forwards("test_removeconstraint", new_state) + self.assertEqual(len(new_state.models["test_removeconstraint", "pony"].options['constraints']), 0) + Pony = new_state.apps.get_model("test_removeconstraint", "Pony") + self.assertEqual(len(Pony._meta.constraints), 0) + with connection.schema_editor() as editor: + lt_operation.database_forwards("test_removeconstraint", editor, project_state, new_state) + Pony.objects.create(pink=100, weight=1.0).delete() # Test reversal with connection.schema_editor() as editor: - operation.database_backwards("test_removeconstraint", editor, new_state, project_state) + gt_operation.database_backwards("test_removeconstraint", editor, new_state, project_state) with self.assertRaises(IntegrityError), transaction.atomic(): Pony.objects.create(pink=1, weight=1.0) # Test deconstruction - definition = operation.deconstruct() + definition = gt_operation.deconstruct() self.assertEqual(definition[0], "RemoveConstraint") self.assertEqual(definition[1], []) - self.assertEqual(definition[2], {'model_name': "Pony", 'name': "pony_test_constraint"}) + self.assertEqual(definition[2], {'model_name': "Pony", 'name': "test_constraint_pony_pink_gt_2"}) def test_alter_model_options(self): """