Fixed #27731 -- Implemented CreateModel/AlterFooOperation reduction.

This should alleviate the side effects of disabling the AlterFooOperation
reduction with RemoveField to fix refs #28862 during migration squashing
because CreateModel can perform a reduction with RemoveField.

Thanks Nick Pope for the review.
This commit is contained in:
Simon Charette 2018-07-14 00:32:09 -04:00 committed by Tim Graham
parent ed7898e1b5
commit 8e3f22f251
4 changed files with 95 additions and 20 deletions

View File

@ -142,6 +142,17 @@ class CreateModel(ModelOperation):
managers=self.managers,
),
]
elif isinstance(operation, FieldRelatedOptionOperation) and self.name_lower == operation.name_lower:
option_value = getattr(operation, operation.option_name)
return [
CreateModel(
self.name,
fields=self.fields,
options={**self.options, **{operation.option_name: option_value}},
bases=self.bases,
managers=self.managers,
),
]
elif isinstance(operation, FieldOperation) and self.name_lower == operation.model_name_lower:
if isinstance(operation, AddField):
return [
@ -167,6 +178,18 @@ class CreateModel(ModelOperation):
),
]
elif isinstance(operation, RemoveField):
options = self.options.copy()
for option_name in ('unique_together', 'index_together'):
option = options.pop(option_name, None)
if option:
option = set(filter(bool, (
tuple(f for f in fields if f != operation.name_lower) for fields in option
)))
if option:
options[option_name] = option
order_with_respect_to = options.get('order_with_respect_to')
if order_with_respect_to == operation.name_lower:
del options['order_with_respect_to']
return [
CreateModel(
self.name,
@ -175,12 +198,23 @@ class CreateModel(ModelOperation):
for n, v in self.fields
if n.lower() != operation.name_lower
],
options=self.options,
options=options,
bases=self.bases,
managers=self.managers,
),
]
elif isinstance(operation, RenameField):
options = self.options.copy()
for option_name in ('unique_together', 'index_together'):
option = options.get(option_name)
if option:
options[option_name] = {
tuple(operation.new_name if f == operation.old_name else f for f in fields)
for fields in option
}
order_with_respect_to = options.get('order_with_respect_to')
if order_with_respect_to == operation.old_name:
options['order_with_respect_to'] = operation.new_name
return [
CreateModel(
self.name,
@ -188,7 +222,7 @@ class CreateModel(ModelOperation):
(operation.new_name if n == operation.old_name else n, v)
for n, v in self.fields
],
options=self.options,
options=options,
bases=self.bases,
managers=self.managers,
),
@ -568,6 +602,8 @@ class AlterIndexTogether(FieldRelatedOptionOperation):
class AlterOrderWithRespectTo(FieldRelatedOptionOperation):
"""Represent a change with the order_with_respect_to option."""
option_name = 'order_with_respect_to'
def __init__(self, name, order_with_respect_to):
self.order_with_respect_to = order_with_respect_to
super().__init__(name)

View File

@ -2075,8 +2075,10 @@ class AutodetectorTests(TestCase):
changes = self.get_changes([], [self.book, self.author_with_book_order_wrt])
# Right number/type of migrations?
self.assertNumberMigrations(changes, 'testapp', 1)
self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "AlterOrderWithRespectTo"])
self.assertOperationAttributes(changes, 'testapp', 0, 1, name="author", order_with_respect_to="book")
self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel"])
self.assertOperationAttributes(
changes, 'testapp', 0, 0, name="Author", options={'order_with_respect_to': 'book'}
)
self.assertNotIn("_order", [name for name, field in changes['testapp'][0].operations[0].fields])
def test_alter_model_managers(self):

View File

@ -1335,7 +1335,7 @@ class SquashMigrationsTests(MigrationTestBase):
out = io.StringIO()
with self.temporary_migration_module(module="migrations.test_migrations"):
call_command("squashmigrations", "migrations", "0002", interactive=False, verbosity=1, stdout=out)
self.assertIn("Optimized from 8 operations to 4 operations.", out.getvalue())
self.assertIn("Optimized from 8 operations to 2 operations.", out.getvalue())
def test_ticket_23799_squashmigrations_no_optimize(self):
"""

View File

@ -594,8 +594,11 @@ class OptimizerTests(SimpleTestCase):
def _test_create_alter_foo_field(self, alter):
"""
CreateModel, AlterFooTogether/AlterOrderWithRespectTo followed by an
add/alter/rename field should optimize to CreateModel and the Alter*
add/alter/rename field should optimize to CreateModel with options.
"""
option_value = getattr(alter, alter.option_name)
options = {alter.option_name: option_value}
# AddField
self.assertOptimizesTo(
[
@ -611,13 +614,12 @@ class OptimizerTests(SimpleTestCase):
("a", models.IntegerField()),
("b", models.IntegerField()),
("c", models.IntegerField()),
]),
alter,
], options=options),
],
)
# AlterField
self.assertDoesNotOptimize(
self.assertOptimizesTo(
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
@ -626,6 +628,12 @@ class OptimizerTests(SimpleTestCase):
alter,
migrations.AlterField("Foo", "b", models.CharField(max_length=255)),
],
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
("b", models.CharField(max_length=255)),
], options=options),
],
)
self.assertOptimizesTo(
@ -643,13 +651,20 @@ class OptimizerTests(SimpleTestCase):
("a", models.IntegerField()),
("b", models.IntegerField()),
("c", models.CharField(max_length=255)),
]),
alter,
], options=options),
],
)
# RenameField
self.assertDoesNotOptimize(
if isinstance(option_value, str):
renamed_options = {alter.option_name: 'c'}
else:
renamed_options = {
alter.option_name: {
tuple('c' if value == 'b' else value for value in item) for item in option_value
}
}
self.assertOptimizesTo(
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
@ -658,6 +673,12 @@ class OptimizerTests(SimpleTestCase):
alter,
migrations.RenameField("Foo", "b", "c"),
],
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
("c", models.IntegerField()),
], options=renamed_options),
],
)
self.assertOptimizesTo(
@ -673,10 +694,8 @@ class OptimizerTests(SimpleTestCase):
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
("b", models.IntegerField()),
]),
alter,
migrations.RenameField("Foo", "b", "c"),
("c", models.IntegerField()),
], options=renamed_options),
],
)
@ -695,13 +714,20 @@ class OptimizerTests(SimpleTestCase):
("a", models.IntegerField()),
("b", models.IntegerField()),
("d", models.IntegerField()),
]),
alter,
], options=options),
],
)
# RemoveField
self.assertDoesNotOptimize(
if isinstance(option_value, str):
removed_options = None
else:
removed_options = {
alter.option_name: {
tuple(value for value in item if value != 'b') for item in option_value
}
}
self.assertOptimizesTo(
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
@ -710,9 +736,14 @@ class OptimizerTests(SimpleTestCase):
alter,
migrations.RemoveField("Foo", "b"),
],
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
], options=removed_options),
]
)
self.assertDoesNotOptimize(
self.assertOptimizesTo(
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
@ -722,6 +753,12 @@ class OptimizerTests(SimpleTestCase):
alter,
migrations.RemoveField("Foo", "c"),
],
[
migrations.CreateModel("Foo", [
("a", models.IntegerField()),
("b", models.IntegerField()),
], options=options),
],
)
def test_create_alter_unique_field(self):