From 8069526ce366c27d657f01e3b172a6b6e10817cb Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 3 Apr 2020 23:05:16 -0400 Subject: [PATCH] Made Operation.references_model/references_field require app_label. This will allow them to drop a ton of logic to deal with null app_label. --- django/db/migrations/operations/base.py | 8 ++--- django/db/migrations/operations/fields.py | 8 ++--- django/db/migrations/operations/models.py | 12 +++---- tests/migrations/test_operations.py | 42 +++++++++++------------ 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/django/db/migrations/operations/base.py b/django/db/migrations/operations/base.py index f02eae20b4..1f5665433c 100644 --- a/django/db/migrations/operations/base.py +++ b/django/db/migrations/operations/base.py @@ -80,10 +80,10 @@ class Operation: """ return "%s: %s" % (self.__class__.__name__, self._constructor_args) - def references_model(self, name, app_label=None): + def references_model(self, name, app_label): """ Return True if there is a chance this operation references the given - model name (as a string), with an optional app label for accuracy. + model name (as a string), with an app label for accuracy. Used for optimization. If in doubt, return True; returning a false positive will merely make the optimizer a little @@ -92,10 +92,10 @@ class Operation: """ return True - def references_field(self, model_name, name, app_label=None): + def references_field(self, model_name, name, app_label): """ Return True if there is a chance this operation references the given - field name, with an optional app label for accuracy. + field name, with an app label for accuracy. Used for optimization. If in doubt, return True. """ diff --git a/django/db/migrations/operations/fields.py b/django/db/migrations/operations/fields.py index 08dfdf430f..1acb2edbd1 100644 --- a/django/db/migrations/operations/fields.py +++ b/django/db/migrations/operations/fields.py @@ -28,7 +28,7 @@ class FieldOperation(Operation): def is_same_field_operation(self, operation): return self.is_same_model_operation(operation) and self.name_lower == operation.name_lower - def references_model(self, name, app_label=None): + def references_model(self, name, app_label): name_lower = name.lower() if name_lower == self.model_name_lower: return True @@ -36,7 +36,7 @@ class FieldOperation(Operation): return field_references_model(self.field, ModelTuple(app_label, name_lower)) return False - def references_field(self, model_name, name, app_label=None): + def references_field(self, model_name, name, app_label): model_name_lower = model_name.lower() # Check if this operation locally references the field. if model_name_lower == self.model_name_lower: @@ -376,8 +376,8 @@ class RenameField(FieldOperation): def describe(self): return "Rename field %s on %s to %s" % (self.old_name, self.model_name, self.new_name) - def references_field(self, model_name, name, app_label=None): - return self.references_model(model_name) and ( + 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 name.lower() == self.new_name_lower ) diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 1947bb1edf..b415bd6417 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -28,7 +28,7 @@ class ModelOperation(Operation): def name_lower(self): return self.name.lower() - def references_model(self, name, app_label=None): + def references_model(self, name, app_label): return name.lower() == self.name_lower def reduce(self, operation, app_label): @@ -99,7 +99,7 @@ class CreateModel(ModelOperation): def describe(self): return "Create %smodel %s" % ("proxy " if self.options.get("proxy", False) else "", self.name) - def references_model(self, name, app_label=None): + def references_model(self, name, app_label): name_lower = name.lower() if name_lower == self.name_lower: return True @@ -265,7 +265,7 @@ class DeleteModel(ModelOperation): if self.allow_migrate_model(schema_editor.connection.alias, model): schema_editor.create_model(model) - def references_model(self, name, app_label=None): + def references_model(self, name, app_label): # The deleted model could be referencing the specified model through # related fields. return True @@ -402,7 +402,7 @@ class RenameModel(ModelOperation): self.new_name_lower, self.old_name_lower = self.old_name_lower, self.new_name_lower self.new_name, self.old_name = self.old_name, self.new_name - def references_model(self, name, app_label=None): + def references_model(self, name, app_label): return ( name.lower() == self.old_name_lower or name.lower() == self.new_name_lower @@ -528,7 +528,7 @@ class AlterTogetherOptionOperation(ModelOptionOperation): def database_backwards(self, app_label, schema_editor, from_state, to_state): return self.database_forwards(app_label, schema_editor, from_state, to_state) - def references_field(self, model_name, name, app_label=None): + def references_field(self, model_name, name, app_label): return ( self.references_model(model_name, app_label) and ( @@ -609,7 +609,7 @@ class AlterOrderWithRespectTo(ModelOptionOperation): def database_backwards(self, app_label, schema_editor, from_state, to_state): self.database_forwards(app_label, schema_editor, from_state, to_state) - def references_field(self, model_name, name, app_label=None): + def references_field(self, model_name, name, app_label): return ( self.references_model(model_name, app_label) and ( diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 2fb633d41e..3908a6466d 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -2917,54 +2917,54 @@ class SwappableOperationTests(OperationTestBase): class TestCreateModel(SimpleTestCase): def test_references_model_mixin(self): - CreateModel('name', [], bases=(Mixin, models.Model)).references_model('other_model') + CreateModel('name', [], bases=(Mixin, models.Model)).references_model('other_model', 'migrations') class FieldOperationTests(SimpleTestCase): def test_references_model(self): operation = FieldOperation('MoDel', 'field', models.ForeignKey('Other', models.CASCADE)) # Model name match. - self.assertIs(operation.references_model('mOdEl'), True) + self.assertIs(operation.references_model('mOdEl', 'migrations'), True) # Referenced field. - self.assertIs(operation.references_model('oTher'), True) + self.assertIs(operation.references_model('oTher', 'migrations'), True) # Doesn't reference. - self.assertIs(operation.references_model('Whatever'), False) + self.assertIs(operation.references_model('Whatever', 'migrations'), False) def test_references_field_by_name(self): operation = FieldOperation('MoDel', 'field', models.BooleanField(default=False)) - self.assertIs(operation.references_field('model', 'field'), True) + self.assertIs(operation.references_field('model', 'field', 'migrations'), True) def test_references_field_by_remote_field_model(self): operation = FieldOperation('Model', 'field', models.ForeignKey('Other', models.CASCADE)) - self.assertIs(operation.references_field('Other', 'whatever'), True) - self.assertIs(operation.references_field('Missing', 'whatever'), False) + self.assertIs(operation.references_field('Other', 'whatever', 'migrations'), True) + self.assertIs(operation.references_field('Missing', 'whatever', 'migrations'), False) def test_references_field_by_from_fields(self): operation = FieldOperation( 'Model', 'field', models.fields.related.ForeignObject('Other', models.CASCADE, ['from'], ['to']) ) - self.assertIs(operation.references_field('Model', 'from'), True) - self.assertIs(operation.references_field('Model', 'to'), False) - self.assertIs(operation.references_field('Other', 'from'), False) - self.assertIs(operation.references_field('Model', 'to'), False) + self.assertIs(operation.references_field('Model', 'from', 'migrations'), True) + self.assertIs(operation.references_field('Model', 'to', 'migrations'), False) + self.assertIs(operation.references_field('Other', 'from', 'migrations'), False) + self.assertIs(operation.references_field('Model', 'to', 'migrations'), False) def test_references_field_by_to_fields(self): operation = FieldOperation('Model', 'field', models.ForeignKey('Other', models.CASCADE, to_field='field')) - self.assertIs(operation.references_field('Other', 'field'), True) - self.assertIs(operation.references_field('Other', 'whatever'), False) - self.assertIs(operation.references_field('Missing', 'whatever'), False) + self.assertIs(operation.references_field('Other', 'field', 'migrations'), True) + self.assertIs(operation.references_field('Other', 'whatever', 'migrations'), False) + self.assertIs(operation.references_field('Missing', 'whatever', 'migrations'), False) def test_references_field_by_through(self): operation = FieldOperation('Model', 'field', models.ManyToManyField('Other', through='Through')) - self.assertIs(operation.references_field('Other', 'whatever'), True) - self.assertIs(operation.references_field('Through', 'whatever'), True) - self.assertIs(operation.references_field('Missing', 'whatever'), False) + self.assertIs(operation.references_field('Other', 'whatever', 'migrations'), True) + self.assertIs(operation.references_field('Through', 'whatever', 'migrations'), True) + self.assertIs(operation.references_field('Missing', 'whatever', 'migrations'), False) def test_reference_field_by_through_fields(self): operation = FieldOperation( 'Model', 'field', models.ManyToManyField('Other', through='Through', through_fields=('first', 'second')) ) - self.assertIs(operation.references_field('Other', 'whatever'), True) - self.assertIs(operation.references_field('Through', 'whatever'), False) - self.assertIs(operation.references_field('Through', 'first'), True) - self.assertIs(operation.references_field('Through', 'second'), True) + self.assertIs(operation.references_field('Other', 'whatever', 'migrations'), True) + self.assertIs(operation.references_field('Through', 'whatever', 'migrations'), False) + self.assertIs(operation.references_field('Through', 'first', 'migrations'), True) + self.assertIs(operation.references_field('Through', 'second', 'migrations'), True)