Refs #22608 -- Made app_label required when optimizing migrations.

This paved the way for the removal of lot of logic when app_label was not specified.

Author: charettes

django/db/migrations/autodetector.py

@@ -369,7 +369,7 @@ def _optimize_migrations(self):
  # Optimize migrations
  for app_label, migrations in self.migrations.items():
  for migration in migrations:
- migration.operations = MigrationOptimizer().optimize(migration.operations, app_label=app_label)
+ migration.operations = MigrationOptimizer().optimize(migration.operations, app_label)
 
  def check_dependency(self, operation, dependency):
  """

django/db/migrations/operations/base.py

@@ -113,7 +113,7 @@ def allow_migrate_model(self, connection_alias, model):
 
  return router.allow_migrate_model(connection_alias, model)
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  """
  Return either a list of operations the actual operation should be
  replaced with or a boolean that indicates whether or not the specified

django/db/migrations/operations/fields.py

@@ -60,9 +60,9 @@ def references_field(self, model_name, name, app_label=None):
  return True
  return False
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  return (
- super().reduce(operation, app_label=app_label) or
+ super().reduce(operation, app_label) or
  not operation.references_field(self.model_name, self.name, app_label)
  )
 
@@ -122,7 +122,7 @@ def database_backwards(self, app_label, schema_editor, from_state, to_state):
  def describe(self):
  return "Add field %s to %s" % (self.name, self.model_name)
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  if isinstance(operation, FieldOperation) and self.is_same_field_operation(operation):
  if isinstance(operation, AlterField):
  return [
@@ -142,7 +142,7 @@ def reduce(self, operation, app_label=None):
  field=self.field,
  ),
  ]
- return super().reduce(operation, app_label=app_label)
+ return super().reduce(operation, app_label)
 
 
 class RemoveField(FieldOperation):
@@ -186,11 +186,11 @@ def database_backwards(self, app_label, schema_editor, from_state, to_state):
  def describe(self):
  return "Remove field %s from %s" % (self.name, self.model_name)
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  from .models import DeleteModel
  if isinstance(operation, DeleteModel) and operation.name_lower == self.model_name_lower:
  return [operation]
- return super().reduce(operation, app_label=app_label)
+ return super().reduce(operation, app_label)
 
 
 class AlterField(FieldOperation):
@@ -256,7 +256,7 @@ def database_backwards(self, app_label, schema_editor, from_state, to_state):
  def describe(self):
  return "Alter field %s on %s" % (self.name, self.model_name)
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  if isinstance(operation, RemoveField) and self.is_same_field_operation(operation):
  return [operation]
  elif isinstance(operation, RenameField) and self.is_same_field_operation(operation):
@@ -268,7 +268,7 @@ def reduce(self, operation, app_label=None):
  field=self.field,
  ),
  ]
- return super().reduce(operation, app_label=app_label)
+ return super().reduce(operation, app_label)
 
 
 class RenameField(FieldOperation):
@@ -382,7 +382,7 @@ def references_field(self, model_name, name, app_label=None):
  name.lower() == self.new_name_lower
  )
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  if (isinstance(operation, RenameField) and
  self.is_same_model_operation(operation) and
  self.new_name_lower == operation.old_name_lower):
@@ -396,6 +396,6 @@ def reduce(self, operation, app_label=None):
  # Skip `FieldOperation.reduce` as we want to run `references_field`
  # against self.new_name.
  return (
- super(FieldOperation, self).reduce(operation, app_label=app_label) or
+ super(FieldOperation, self).reduce(operation, app_label) or
  not operation.references_field(self.model_name, self.new_name, app_label)
  )

django/db/migrations/operations/models.py

@@ -31,9 +31,9 @@ def name_lower(self):
  def references_model(self, name, app_label=None):
  return name.lower() == self.name_lower
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  return (
- super().reduce(operation, app_label=app_label) or
+ super().reduce(operation, app_label) or
  not operation.references_model(self.name, app_label)
  )
 
@@ -117,7 +117,7 @@ def references_model(self, name, app_label=None):
  return True
  return False
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  if (isinstance(operation, DeleteModel) and
  self.name_lower == operation.name_lower and
  not self.options.get("proxy", False)):
@@ -236,7 +236,7 @@ def reduce(self, operation, app_label=None):
  managers=self.managers,
  ),
  ]
- return super().reduce(operation, app_label=app_label)
+ return super().reduce(operation, app_label)
 
 
 class DeleteModel(ModelOperation):
@@ -411,7 +411,7 @@ def references_model(self, name, app_label=None):
  def describe(self):
  return "Rename model %s to %s" % (self.old_name, self.new_name)
 
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  if (isinstance(operation, RenameModel) and
  self.new_name_lower == operation.old_name_lower):
  return [
@@ -423,16 +423,16 @@ def reduce(self, operation, app_label=None):
  # Skip `ModelOperation.reduce` as we want to run `references_model`
  # against self.new_name.
  return (
- super(ModelOperation, self).reduce(operation, app_label=app_label) or
+ super(ModelOperation, self).reduce(operation, app_label) or
  not operation.references_model(self.new_name, app_label)
  )
 
 
 class ModelOptionOperation(ModelOperation):
- def reduce(self, operation, app_label=None):
+ def reduce(self, operation, app_label):
  if isinstance(operation, (self.__class__, DeleteModel)) and self.name_lower == operation.name_lower:
  return [operation]
- return super().reduce(operation, app_label=app_label)
+ return super().reduce(operation, app_label)
 
 
 class AlterModelTable(ModelOptionOperation):

django/db/migrations/optimizer.py

@@ -9,7 +9,7 @@ class MigrationOptimizer:
  nothing.
  """
 
- def optimize(self, operations, app_label=None):
+ def optimize(self, operations, app_label):
  """
  Main optimization entry point. Pass in a list of Operation instances,
  get out a new list of Operation instances.
@@ -25,11 +25,10 @@ def optimize(self, operations, app_label=None):
  The inner loop is run until the starting list is the same as the result
  list, and then the result is returned. This means that operation
  optimization must be stable and always return an equal or shorter list.
-
- The app_label argument is optional, but if you pass it you'll get more
- efficient optimization.
  """
  # Internal tracking variable for test assertions about # of loops
+ if app_label is None:
+ raise TypeError('app_label must be a str.')
  self._iterations = 0
  while True:
  result = self.optimize_inner(operations, app_label)
@@ -38,7 +37,7 @@ def optimize(self, operations, app_label=None):
  return result
  operations = result
 
- def optimize_inner(self, operations, app_label=None):
+ def optimize_inner(self, operations, app_label):
  """Inner optimization loop."""
  new_operations = []
  for i, operation in enumerate(operations):

tests/migrations/test_optimizer.py

@@ -23,7 +23,7 @@ def serialize(self, value):
  return serializer_factory(value).serialize()[0]
 
  def assertOptimizesTo(self, operations, expected, exact=None, less_than=None, app_label=None):
- result, iterations = self.optimize(operations, app_label)
+ result, iterations = self.optimize(operations, app_label or 'migrations')
  result = [self.serialize(f) for f in result]
  expected = [self.serialize(f) for f in expected]
  self.assertEqual(expected, result)
@@ -39,6 +39,11 @@ def assertOptimizesTo(self, operations, expected, exact=None, less_than=None, ap
  def assertDoesNotOptimize(self, operations, **kwargs):
  self.assertOptimizesTo(operations, operations, **kwargs)
 
+ def test_none_app_label(self):
+ optimizer = MigrationOptimizer()
+ with self.assertRaisesMessage(TypeError, 'app_label must be a str'):
+ optimizer.optimize([], None)
+
  def test_single(self):
  """
  The optimizer does nothing on a single operation,
@@ -212,16 +217,8 @@ def test_optimize_through_create(self):
  ],
  [],
  )
- # This should not work - FK should block it
- self.assertDoesNotOptimize(
- [
- migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
- migrations.CreateModel("Bar", [("other", models.ForeignKey("testapp.Foo", models.CASCADE))]),
- migrations.DeleteModel("Foo"),
- ],
- )
- # The same operations should be optimized if app_label is specified and
- # a FK references a model from the other app.
+ # Operations should be optimized if the FK references a model from the
+ # other app.
  self.assertOptimizesTo(
  [
  migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
@@ -235,6 +232,13 @@ def test_optimize_through_create(self):
  )
  # But it shouldn't work if a FK references a model with the same
  # app_label.
+ self.assertDoesNotOptimize(
+ [
+ migrations.CreateModel('Foo', [('name', models.CharField(max_length=255))]),
+ migrations.CreateModel('Bar', [('other', models.ForeignKey('Foo', models.CASCADE))]),
+ migrations.DeleteModel('Foo'),
+ ],
+ )
  self.assertDoesNotOptimize(
  [
  migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
@@ -244,12 +248,20 @@ def test_optimize_through_create(self):
  app_label="testapp",
  )
  # This should not work - bases should block it
+ self.assertDoesNotOptimize(
+ [
+ migrations.CreateModel('Foo', [('name', models.CharField(max_length=255))]),
+ migrations.CreateModel('Bar', [('size', models.IntegerField())], bases=('Foo',)),
+ migrations.DeleteModel('Foo'),
+ ],
+ )
  self.assertDoesNotOptimize(
  [
  migrations.CreateModel("Foo", [("name", models.CharField(max_length=255))]),
  migrations.CreateModel("Bar", [("size", models.IntegerField())], bases=("testapp.Foo",)),
  migrations.DeleteModel("Foo"),
  ],
+ app_label='testapp',
  )
  # The same operations should be optimized if app_label and none of
  # bases belong to that app.
@@ -293,6 +305,7 @@ def test_optimize_through_create(self):
  ('reviewer', models.ForeignKey('test_app.Reviewer', models.CASCADE)),
  ]),
  ],
+ app_label='test_app',
  )
 
  def test_create_model_add_field(self):