Fixed #26064 -- Moved operation reduction logic to their own class.

Thanks to Markus Holtermann and Tim Graham for their review.
This commit is contained in:
Simon Charette 2016-01-09 02:36:09 -05:00
parent 6b258befc2
commit 49f4c9f4c6
4 changed files with 248 additions and 397 deletions

View File

@ -111,6 +111,14 @@ class Operation(object):
return router.allow_migrate_model(connection_alias, model)
def reduce(self, operation, in_between, app_label=None):
"""
Return either a list of operations the actual operation should be
replaced with or a boolean that indicates whether or not the specified
operation can be optimized across.
"""
return False
def __repr__(self):
return "<%s %s%s>" % (
self.__class__.__name__,

View File

@ -7,24 +7,38 @@ from django.utils.functional import cached_property
from .base import Operation
class AddField(Operation):
"""
Adds a field to a model.
"""
def __init__(self, model_name, name, field, preserve_default=True):
class FieldOperation(Operation):
def __init__(self, model_name, name):
self.model_name = model_name
self.name = name
self.field = field
self.preserve_default = preserve_default
@cached_property
def model_name_lower(self):
return self.model_name.lower()
@cached_property
def name_lower(self):
return self.name.lower()
@cached_property
def model_name_lower(self):
return self.model_name.lower()
def is_same_model_operation(self, operation):
return self.model_name_lower == operation.model_name_lower
def is_same_field_operation(self, operation):
return self.is_same_model_operation(operation) and self.name_lower == operation.name_lower
def reduce(self, operation, in_between, app_label=None):
return not operation.references_field(self.model_name, self.name, app_label)
class AddField(FieldOperation):
"""
Adds a field to a model.
"""
def __init__(self, model_name, name, field, preserve_default=True):
self.field = field
self.preserve_default = preserve_default
super(AddField, self).__init__(model_name, name)
def deconstruct(self):
kwargs = {
@ -78,24 +92,34 @@ class AddField(Operation):
def references_field(self, model_name, name, app_label=None):
return self.references_model(model_name) and name.lower() == self.name_lower
def reduce(self, operation, in_between, app_label=None):
if isinstance(operation, FieldOperation) and self.is_same_field_operation(operation):
if isinstance(operation, AlterField):
return [
AddField(
model_name=self.model_name,
name=operation.name,
field=operation.field,
),
]
elif isinstance(operation, RemoveField):
return []
elif isinstance(operation, RenameField):
return [
AddField(
model_name=self.model_name,
name=operation.new_name,
field=self.field,
),
]
return super(AddField, self).reduce(operation, in_between, app_label=app_label)
class RemoveField(Operation):
class RemoveField(FieldOperation):
"""
Removes a field from a model.
"""
def __init__(self, model_name, name):
self.model_name = model_name
self.name = name
@cached_property
def name_lower(self):
return self.name.lower()
@cached_property
def model_name_lower(self):
return self.model_name.lower()
def deconstruct(self):
kwargs = {
'model_name': self.model_name,
@ -136,24 +160,15 @@ class RemoveField(Operation):
return self.references_model(model_name) and name.lower() == self.name_lower
class AlterField(Operation):
class AlterField(FieldOperation):
"""
Alters a field's database column (e.g. null, max_length) to the provided new field
"""
def __init__(self, model_name, name, field, preserve_default=True):
self.model_name = model_name
self.name = name
self.field = field
self.preserve_default = preserve_default
@cached_property
def name_lower(self):
return self.name.lower()
@cached_property
def model_name_lower(self):
return self.model_name.lower()
super(AlterField, self).__init__(model_name, name)
def deconstruct(self):
kwargs = {
@ -214,16 +229,30 @@ class AlterField(Operation):
def references_field(self, model_name, name, app_label=None):
return self.references_model(model_name) and name.lower() == self.name_lower
def reduce(self, operation, in_between, app_label=None):
if isinstance(operation, RemoveField) and self.is_same_field_operation(operation):
return [operation]
elif isinstance(operation, RenameField) and self.is_same_field_operation(operation):
return [
operation,
AlterField(
model_name=self.model_name,
name=operation.new_name,
field=self.field,
),
]
return super(AlterField, self).reduce(operation, in_between, app_label=app_label)
class RenameField(Operation):
class RenameField(FieldOperation):
"""
Renames a field on the model. Might affect db_column too.
"""
def __init__(self, model_name, old_name, new_name):
self.model_name = model_name
self.old_name = old_name
self.new_name = new_name
super(RenameField, self).__init__(model_name, old_name)
@cached_property
def old_name_lower(self):
@ -233,10 +262,6 @@ class RenameField(Operation):
def new_name_lower(self):
return self.new_name.lower()
@cached_property
def model_name_lower(self):
return self.model_name.lower()
def deconstruct(self):
kwargs = {
'model_name': self.model_name,
@ -296,3 +321,16 @@ class RenameField(Operation):
name.lower() == self.old_name_lower or
name.lower() == self.new_name_lower
)
def reduce(self, operation, in_between, app_label=None):
if (isinstance(operation, RenameField) and
self.is_same_model_operation(operation) and
self.new_name_lower == operation.old_name_lower):
return [
RenameField(
self.model_name,
self.old_name,
operation.new_name,
),
]
return not operation.references_field(self.model_name, self.new_name, app_label)

View File

@ -7,8 +7,24 @@ from django.db.models.options import normalize_together
from django.utils import six
from django.utils.functional import cached_property
from .fields import (
AddField, AlterField, FieldOperation, RemoveField, RenameField,
)
class CreateModel(Operation):
class ModelOperation(Operation):
def __init__(self, name):
self.name = name
@cached_property
def name_lower(self):
return self.name.lower()
def reduce(self, operation, in_between, app_label=None):
return not operation.references_model(self.name, app_label)
class CreateModel(ModelOperation):
"""
Create a model's table.
"""
@ -16,15 +32,11 @@ class CreateModel(Operation):
serialization_expand_args = ['fields', 'options', 'managers']
def __init__(self, name, fields, options=None, bases=None, managers=None):
self.name = name
self.fields = fields
self.options = options or {}
self.bases = bases or (models.Model,)
self.managers = managers or []
@cached_property
def name_lower(self):
return self.name.lower()
super(CreateModel, self).__init__(name)
def deconstruct(self):
kwargs = {
@ -83,19 +95,102 @@ class CreateModel(Operation):
return True
return False
def model_to_key(self, model):
"""
Take either a model class or an "app_label.ModelName" string
and return (app_label, object_name).
"""
if isinstance(model, six.string_types):
return model.split(".", 1)
else:
return model._meta.app_label, model._meta.object_name
class DeleteModel(Operation):
def reduce(self, operation, in_between, app_label=None):
if (isinstance(operation, DeleteModel) and
self.name_lower == operation.name_lower and
not self.options.get("proxy", False)):
return []
elif isinstance(operation, RenameModel) and self.name_lower == operation.old_name_lower:
return [
CreateModel(
operation.new_name,
fields=self.fields,
options=self.options,
bases=self.bases,
managers=self.managers,
),
]
elif isinstance(operation, FieldOperation) and self.name_lower == operation.model_name_lower:
if isinstance(operation, AddField):
# Don't allow optimizations of FKs through models they reference
if hasattr(operation.field, "remote_field") and operation.field.remote_field:
for between in in_between:
# Check that it doesn't point to the model
app_label, object_name = self.model_to_key(operation.field.remote_field.model)
if between.references_model(object_name, app_label):
return False
# Check that it's not through the model
if getattr(operation.field.remote_field, "through", None):
app_label, object_name = self.model_to_key(operation.field.remote_field.through)
if between.references_model(object_name, app_label):
return False
return [
CreateModel(
self.name,
fields=self.fields + [(operation.name, operation.field)],
options=self.options,
bases=self.bases,
managers=self.managers,
),
]
elif isinstance(operation, AlterField):
return [
CreateModel(
self.name,
fields=[
(n, operation.field if n == operation.name else v)
for n, v in self.fields
],
options=self.options,
bases=self.bases,
managers=self.managers,
),
]
elif isinstance(operation, RemoveField):
return [
CreateModel(
self.name,
fields=[
(n, v)
for n, v in self.fields
if n.lower() != operation.name_lower
],
options=self.options,
bases=self.bases,
managers=self.managers,
),
]
elif isinstance(operation, RenameField):
return [
CreateModel(
self.name,
fields=[
(operation.new_name if n == operation.old_name else n, v)
for n, v in self.fields
],
options=self.options,
bases=self.bases,
managers=self.managers,
),
]
return super(CreateModel, self).reduce(operation, in_between, app_label=app_label)
class DeleteModel(ModelOperation):
"""
Drops a model's table.
"""
def __init__(self, name):
self.name = name
@cached_property
def name_lower(self):
return self.name.lower()
def deconstruct(self):
kwargs = {
'name': self.name,
@ -126,7 +221,7 @@ class DeleteModel(Operation):
return "Delete model %s" % (self.name, )
class RenameModel(Operation):
class RenameModel(ModelOperation):
"""
Renames a model.
"""
@ -134,6 +229,7 @@ class RenameModel(Operation):
def __init__(self, old_name, new_name):
self.old_name = old_name
self.new_name = new_name
super(RenameModel, self).__init__(old_name)
@cached_property
def old_name_lower(self):
@ -260,19 +356,26 @@ class RenameModel(Operation):
def describe(self):
return "Rename model %s to %s" % (self.old_name, self.new_name)
def reduce(self, operation, in_between, app_label=None):
if (isinstance(operation, RenameModel) and
self.new_name_lower == operation.old_name_lower):
return [
RenameModel(
self.old_name,
operation.new_name,
),
]
return not operation.references_model(self.new_name, app_label)
class AlterModelTable(Operation):
class AlterModelTable(ModelOperation):
"""
Renames a model's table
"""
def __init__(self, name, table):
self.name = name
self.table = table
@cached_property
def name_lower(self):
return self.name.lower()
super(AlterModelTable, self).__init__(name)
def deconstruct(self):
kwargs = {
@ -316,8 +419,29 @@ class AlterModelTable(Operation):
def describe(self):
return "Rename table for %s to %s" % (self.name, self.table)
def reduce(self, operation, in_between, app_label=None):
if isinstance(operation, (AlterModelTable, DeleteModel)) and self.name_lower == operation.name_lower:
return [operation]
return super(AlterModelTable, self).reduce(operation, in_between, app_label=app_label)
class AlterUniqueTogether(Operation):
class ModelOptionOperation(ModelOperation):
def reduce(self, operation, in_between, app_label=None):
if isinstance(operation, (self.__class__, DeleteModel)) and self.name_lower == operation.name_lower:
return [operation]
return super(ModelOptionOperation, self).reduce(operation, in_between, app_label=app_label)
class FieldRelatedOptionOperation(ModelOptionOperation):
def reduce(self, operation, in_between, app_label=None):
if (isinstance(operation, FieldOperation) and
self.name_lower == operation.model_name_lower and
not self.references_field(operation.model_name, operation.name)):
return [operation, self]
return super(FieldRelatedOptionOperation, self).reduce(operation, in_between, app_label=app_label)
class AlterUniqueTogether(FieldRelatedOptionOperation):
"""
Changes the value of unique_together to the target one.
Input value of unique_together must be a set of tuples.
@ -325,13 +449,9 @@ class AlterUniqueTogether(Operation):
option_name = "unique_together"
def __init__(self, name, unique_together):
self.name = name
unique_together = normalize_together(unique_together)
self.unique_together = set(tuple(cons) for cons in unique_together)
@cached_property
def name_lower(self):
return self.name.lower()
super(AlterUniqueTogether, self).__init__(name)
def deconstruct(self):
kwargs = {
@ -378,7 +498,7 @@ class AlterUniqueTogether(Operation):
return "Alter %s for %s (%s constraint(s))" % (self.option_name, self.name, len(self.unique_together or ''))
class AlterIndexTogether(Operation):
class AlterIndexTogether(FieldRelatedOptionOperation):
"""
Changes the value of index_together to the target one.
Input value of index_together must be a set of tuples.
@ -386,13 +506,9 @@ class AlterIndexTogether(Operation):
option_name = "index_together"
def __init__(self, name, index_together):
self.name = name
index_together = normalize_together(index_together)
self.index_together = set(tuple(cons) for cons in index_together)
@cached_property
def name_lower(self):
return self.name.lower()
super(AlterIndexTogether, self).__init__(name)
def deconstruct(self):
kwargs = {
@ -439,18 +555,14 @@ class AlterIndexTogether(Operation):
return "Alter %s for %s (%s constraint(s))" % (self.option_name, self.name, len(self.index_together or ''))
class AlterOrderWithRespectTo(Operation):
class AlterOrderWithRespectTo(FieldRelatedOptionOperation):
"""
Represents a change with the order_with_respect_to option.
"""
def __init__(self, name, order_with_respect_to):
self.name = name
self.order_with_respect_to = order_with_respect_to
@cached_property
def name_lower(self):
return self.name.lower()
super(AlterOrderWithRespectTo, self).__init__(name)
def deconstruct(self):
kwargs = {
@ -505,7 +617,7 @@ class AlterOrderWithRespectTo(Operation):
return "Set order_with_respect_to on %s to %s" % (self.name, self.order_with_respect_to)
class AlterModelOptions(Operation):
class AlterModelOptions(ModelOptionOperation):
"""
Sets new model options that don't directly affect the database schema
(like verbose_name, permissions, ordering). Python code in migrations
@ -525,12 +637,8 @@ class AlterModelOptions(Operation):
]
def __init__(self, name, options):
self.name = name
self.options = options
@cached_property
def name_lower(self):
return self.name.lower()
super(AlterModelOptions, self).__init__(name)
def deconstruct(self):
kwargs = {
@ -565,7 +673,7 @@ class AlterModelOptions(Operation):
return "Change Meta options on %s" % (self.name, )
class AlterModelManagers(Operation):
class AlterModelManagers(ModelOptionOperation):
"""
Alters the model's managers
"""
@ -573,12 +681,8 @@ class AlterModelManagers(Operation):
serialization_expand_args = ['managers']
def __init__(self, name, managers):
self.name = name
self.managers = managers
@cached_property
def name_lower(self):
return self.name.lower()
super(AlterModelManagers, self).__init__(name)
def deconstruct(self):
return (

View File

@ -1,12 +1,5 @@
from __future__ import unicode_literals
from django.db.migrations import (
AddField, AlterField, AlterIndexTogether, AlterModelTable,
AlterOrderWithRespectTo, AlterUniqueTogether, CreateModel, DeleteModel,
RemoveField, RenameField, RenameModel,
)
from django.utils import six
class MigrationOptimizer(object):
"""
@ -19,63 +12,6 @@ class MigrationOptimizer(object):
nothing.
"""
def __init__(self):
self.model_level_operations = (
CreateModel,
AlterModelTable,
AlterUniqueTogether,
AlterIndexTogether,
AlterOrderWithRespectTo,
)
self.field_level_operations = (
AddField,
AlterField,
)
self.reduce_methods = {
# (model operation, model operation)
(CreateModel, DeleteModel): self.reduce_create_model_delete_model,
(CreateModel, RenameModel): self.reduce_create_model_rename_model,
(RenameModel, RenameModel): self.reduce_rename_model_rename_model,
(AlterIndexTogether, AlterIndexTogether): self.reduce_alter_model_alter_model,
(AlterModelTable, AlterModelTable): self.reduce_alter_model_alter_model,
(AlterOrderWithRespectTo, AlterOrderWithRespectTo): self.reduce_alter_model_alter_model,
(AlterUniqueTogether, AlterUniqueTogether): self.reduce_alter_model_alter_model,
(AlterIndexTogether, DeleteModel): self.reduce_alter_model_delete_model,
(AlterModelTable, DeleteModel): self.reduce_alter_model_delete_model,
(AlterOrderWithRespectTo, DeleteModel): self.reduce_alter_model_delete_model,
(AlterUniqueTogether, DeleteModel): self.reduce_alter_model_delete_model,
# (model operation, field operation)
(CreateModel, AddField): self.reduce_create_model_add_field,
(CreateModel, AlterField): self.reduce_create_model_alter_field,
(CreateModel, RemoveField): self.reduce_create_model_remove_field,
(CreateModel, RenameField): self.reduce_create_model_rename_field,
(AlterIndexTogether, AddField): self.reduce_alter_model_addalterremove_field,
(AlterIndexTogether, AlterField): self.reduce_alter_model_addalterremove_field,
(AlterIndexTogether, RemoveField): self.reduce_alter_model_addalterremove_field,
(AlterOrderWithRespectTo, AddField): self.reduce_alter_model_addalterremove_field,
(AlterOrderWithRespectTo, AlterField): self.reduce_alter_model_addalterremove_field,
(AlterOrderWithRespectTo, RemoveField): self.reduce_alter_model_addalterremove_field,
(AlterUniqueTogether, AddField): self.reduce_alter_model_addalterremove_field,
(AlterUniqueTogether, AlterField): self.reduce_alter_model_addalterremove_field,
(AlterUniqueTogether, RemoveField): self.reduce_alter_model_addalterremove_field,
(AlterIndexTogether, RenameField): self.reduce_alter_model_rename_field,
(AlterOrderWithRespectTo, RenameField): self.reduce_alter_model_rename_field,
(AlterUniqueTogether, RenameField): self.reduce_alter_model_rename_field,
# (field operation, field operation)
(AddField, AlterField): self.reduce_add_field_alter_field,
(AddField, RemoveField): self.reduce_add_field_remove_field,
(AddField, RenameField): self.reduce_add_field_rename_field,
(AlterField, RemoveField): self.reduce_alter_field_remove_field,
(AlterField, RenameField): self.reduce_alter_field_rename_field,
(RenameField, RenameField): self.reduce_rename_field_rename_field,
}
def optimize(self, operations, app_label=None):
"""
Main optimization entry point. Pass in a list of Operation instances,
@ -113,253 +49,18 @@ class MigrationOptimizer(object):
for i, operation in enumerate(operations):
# Compare it to each operation after it
for j, other in enumerate(operations[i + 1:]):
result = self.reduce(operation, other, operations[i + 1:i + j + 1])
if result is not None:
in_between = operations[i + 1:i + j + 1]
result = operation.reduce(other, in_between, app_label)
if isinstance(result, list):
# Optimize! Add result, then remaining others, then return
new_operations.extend(result)
new_operations.extend(operations[i + 1:i + 1 + j])
new_operations.extend(in_between)
new_operations.extend(operations[i + j + 2:])
return new_operations
if not self.can_optimize_through(operation, other, app_label):
if not result:
# We can't optimize across `other`.
new_operations.append(operation)
break
else:
new_operations.append(operation)
return new_operations
# REDUCTION
def reduce(self, operation, other, in_between=None):
"""
Either returns a list of zero, one or two operations,
or None, meaning this pair cannot be optimized.
"""
method = self.reduce_methods.get((type(operation), type(other)))
if method:
return method(operation, other, in_between or [])
return None
def model_to_key(self, model):
"""
Takes either a model class or a "appname.ModelName" string
and returns (appname, modelname)
"""
if isinstance(model, six.string_types):
return model.split(".", 1)
else:
return (
model._meta.app_label,
model._meta.object_name,
)
# REDUCE METHODS: (MODEL OPERATION, MODEL OPERATION)
def reduce_create_model_delete_model(self, operation, other, in_between):
"""
Folds a CreateModel and a DeleteModel into nothing.
"""
if (operation.name_lower == other.name_lower and
not operation.options.get("proxy", False)):
return []
def reduce_create_model_rename_model(self, operation, other, in_between):
"""
Folds a model rename into its create
"""
if operation.name_lower == other.old_name_lower:
return [
CreateModel(
other.new_name,
fields=operation.fields,
options=operation.options,
bases=operation.bases,
managers=operation.managers,
)
]
def reduce_rename_model_rename_model(self, operation, other, in_between):
"""
Folds a model rename into another one
"""
if operation.new_name_lower == other.old_name_lower:
return [
RenameModel(
operation.old_name,
other.new_name,
)
]
def reduce_alter_model_alter_model(self, operation, other, in_between):
"""
Folds two AlterModelTable, AlterFooTogether, or AlterOrderWithRespectTo
operations into the latter.
"""
if operation.name_lower == other.name_lower:
return [other]
def reduce_alter_model_delete_model(self, operation, other, in_between):
"""
Folds an AlterModelSomething and a DeleteModel into just delete.
"""
if operation.name_lower == other.name_lower:
return [other]
# REDUCE METHODS: (MODEL OPERATION, FIELD OPERATION)
def reduce_create_model_add_field(self, operation, other, in_between):
if operation.name_lower == other.model_name_lower:
# Don't allow optimizations of FKs through models they reference
if hasattr(other.field, "remote_field") and other.field.remote_field:
for between in in_between:
# Check that it doesn't point to the model
app_label, object_name = self.model_to_key(other.field.remote_field.model)
if between.references_model(object_name, app_label):
return None
# Check that it's not through the model
if getattr(other.field.remote_field, "through", None):
app_label, object_name = self.model_to_key(other.field.remote_field.through)
if between.references_model(object_name, app_label):
return None
# OK, that's fine
return [
CreateModel(
operation.name,
fields=operation.fields + [(other.name, other.field)],
options=operation.options,
bases=operation.bases,
managers=operation.managers,
)
]
def reduce_create_model_alter_field(self, operation, other, in_between):
if operation.name_lower == other.model_name_lower:
return [
CreateModel(
operation.name,
fields=[
(n, other.field if n == other.name else v)
for n, v in operation.fields
],
options=operation.options,
bases=operation.bases,
managers=operation.managers,
)
]
def reduce_create_model_remove_field(self, operation, other, in_between):
if operation.name_lower == other.model_name_lower:
return [
CreateModel(
operation.name,
fields=[
(n, v)
for n, v in operation.fields
if n.lower() != other.name_lower
],
options=operation.options,
bases=operation.bases,
managers=operation.managers,
)
]
def reduce_create_model_rename_field(self, operation, other, in_between):
if operation.name_lower == other.model_name_lower:
return [
CreateModel(
operation.name,
fields=[
(other.new_name if n == other.old_name else n, v)
for n, v in operation.fields
],
options=operation.options,
bases=operation.bases,
managers=operation.managers,
)
]
def reduce_alter_model_addalterremove_field(self, operation, other, in_between):
if (operation.name_lower == other.model_name_lower and
not operation.references_field(other.model_name, other.name)):
return [other, operation]
def reduce_alter_model_rename_field(self, operation, other, in_between):
if (operation.name_lower == other.model_name_lower and
not operation.references_field(other.model_name, other.old_name)):
return [other, operation]
# REDUCE METHODS: (FIELD OPERATION, FIELD OPERATION)
def reduce_add_field_alter_field(self, operation, other, in_between):
if (operation.model_name_lower == other.model_name_lower and
operation.name_lower == other.name_lower):
return [
AddField(
model_name=operation.model_name,
name=operation.name,
field=other.field,
)
]
def reduce_add_field_remove_field(self, operation, other, in_between):
if (operation.model_name_lower == other.model_name_lower and
operation.name_lower == other.name_lower):
return []
def reduce_add_field_rename_field(self, operation, other, in_between):
if (operation.model_name_lower == other.model_name_lower and
operation.name_lower == other.old_name_lower):
return [
AddField(
model_name=operation.model_name,
name=other.new_name,
field=operation.field,
)
]
def reduce_alter_field_remove_field(self, operation, other, in_between):
if (operation.model_name_lower == other.model_name_lower and
operation.name_lower == other.name_lower):
return [other]
def reduce_alter_field_rename_field(self, operation, other, in_between):
if (operation.model_name_lower == other.model_name_lower and
operation.name_lower == other.old_name_lower):
return [
other,
AlterField(
model_name=operation.model_name,
name=other.new_name,
field=operation.field,
),
]
def reduce_rename_field_rename_field(self, operation, other, in_between):
if (operation.model_name_lower == other.model_name_lower and
operation.new_name_lower == other.old_name_lower):
return [
RenameField(
operation.model_name,
operation.old_name,
other.new_name,
),
]
# THROUGH CHECKS
def can_optimize_through(self, operation, other, app_label=None):
"""
Returns True if it's possible to optimize 'operation' with something
the other side of 'other'. This is possible if, for example, they
affect different models.
"""
# If it's a model level operation, let it through if there's
# nothing that looks like a reference to us in 'other'.
if isinstance(operation, self.model_level_operations):
if not other.references_model(operation.name, app_label):
return True
# If it's field level, only let it through things that don't reference
# the field (which includes not referencing the model)
if isinstance(operation, self.field_level_operations):
if not other.references_field(operation.model_name, operation.name, app_label):
return True
return False