Fixed #33366 -- Fixed case handling with swappable setting detection in migrations autodetector.

The migration framework uniquely identifies models by case insensitive
labels composed of their app label and model names and so does the app
registry in most of its methods (e.g. AppConfig.get_model) but it
wasn't the case for get_swappable_settings_name() until this change.

This likely slipped under the radar for so long and only regressed in
b9df2b74b9 because prior to the changes
related to the usage of model states instead of rendered models in the
auto-detector the exact value settings value was never going through a
case folding hoop.

Thanks Andrew Chen Wang for the report and Keryn Knight for the
investigation.
This commit is contained in:
Simon Charette 2021-12-15 19:36:08 -05:00 committed by Mariusz Felisiak
parent 40165eecc4
commit 4328970780
4 changed files with 43 additions and 2 deletions

View File

@ -286,13 +286,14 @@ class Apps:
change after Django has loaded the settings, there is no reason to get change after Django has loaded the settings, there is no reason to get
the respective settings attribute over and over again. the respective settings attribute over and over again.
""" """
to_string = to_string.lower()
for model in self.get_models(include_swapped=True): for model in self.get_models(include_swapped=True):
swapped = model._meta.swapped swapped = model._meta.swapped
# Is this model swapped out for the model given by to_string? # Is this model swapped out for the model given by to_string?
if swapped and swapped == to_string: if swapped and swapped.lower() == to_string:
return model._meta.swappable return model._meta.swappable
# Is this model swappable and the one given by to_string? # Is this model swappable and the one given by to_string?
if model._meta.swappable and model._meta.label == to_string: if model._meta.swappable and model._meta.label_lower == to_string:
return model._meta.swappable return model._meta.swappable
return None return None

View File

@ -19,3 +19,7 @@ Bugfixes
* Relaxed the check added in Django 4.0 to reallow use of a duck-typed * Relaxed the check added in Django 4.0 to reallow use of a duck-typed
``HttpRequest`` in ``django.views.decorators.cache.cache_control()`` and ``HttpRequest`` in ``django.views.decorators.cache.cache_control()`` and
``never_cache()`` decorators (:ticket:`33350`). ``never_cache()`` decorators (:ticket:`33350`).
* Fixed a regression in Django 4.0 that caused creating bogus migrations for
models that reference swappable models such as ``auth.User``
(:ticket:`33366`).

View File

@ -211,6 +211,13 @@ class FieldDeconstructionTests(SimpleTestCase):
self.assertEqual(args, []) self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.user", "on_delete": models.CASCADE}) self.assertEqual(kwargs, {"to": "auth.user", "on_delete": models.CASCADE})
self.assertEqual(kwargs['to'].setting_name, "AUTH_USER_MODEL") self.assertEqual(kwargs['to'].setting_name, "AUTH_USER_MODEL")
# Swap detection for lowercase swappable model.
field = models.ForeignKey('auth.user', models.CASCADE)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, 'django.db.models.ForeignKey')
self.assertEqual(args, [])
self.assertEqual(kwargs, {'to': 'auth.user', 'on_delete': models.CASCADE})
self.assertEqual(kwargs['to'].setting_name, 'AUTH_USER_MODEL')
# Test nonexistent (for now) model # Test nonexistent (for now) model
field = models.ForeignKey("something.Else", models.CASCADE) field = models.ForeignKey("something.Else", models.CASCADE)
name, path, args, kwargs = field.deconstruct() name, path, args, kwargs = field.deconstruct()
@ -273,6 +280,19 @@ class FieldDeconstructionTests(SimpleTestCase):
self.assertEqual(kwargs, {"to": "auth.permission", "on_delete": models.CASCADE}) self.assertEqual(kwargs, {"to": "auth.permission", "on_delete": models.CASCADE})
self.assertEqual(kwargs['to'].setting_name, "AUTH_USER_MODEL") self.assertEqual(kwargs['to'].setting_name, "AUTH_USER_MODEL")
# Model names are case-insensitive.
with isolate_lru_cache(apps.get_swappable_settings_name):
# It doesn't matter that we swapped out user for permission;
# there's no validation. We just want to check the setting stuff
# works.
field = models.ForeignKey('auth.permission', models.CASCADE)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, 'django.db.models.ForeignKey')
self.assertEqual(args, [])
self.assertEqual(kwargs, {'to': 'auth.permission', 'on_delete': models.CASCADE})
self.assertEqual(kwargs['to'].setting_name, 'AUTH_USER_MODEL')
def test_one_to_one(self): def test_one_to_one(self):
# Test basic pointing # Test basic pointing
from django.contrib.auth.models import Permission from django.contrib.auth.models import Permission

View File

@ -1965,6 +1965,22 @@ class AutodetectorTests(TestCase):
self.assertOperationAttributes(changes, 'testapp', 0, 0, name="Author") self.assertOperationAttributes(changes, 'testapp', 0, 0, name="Author")
self.assertMigrationDependencies(changes, 'testapp', 0, [("__setting__", "AUTH_USER_MODEL")]) self.assertMigrationDependencies(changes, 'testapp', 0, [("__setting__", "AUTH_USER_MODEL")])
def test_swappable_lowercase(self):
model_state = ModelState('testapp', 'Document', [
('id', models.AutoField(primary_key=True)),
('owner', models.ForeignKey(
settings.AUTH_USER_MODEL.lower(), models.CASCADE,
)),
])
with isolate_lru_cache(apps.get_swappable_settings_name):
changes = self.get_changes([], [model_state])
self.assertNumberMigrations(changes, 'testapp', 1)
self.assertOperationTypes(changes, 'testapp', 0, ['CreateModel'])
self.assertOperationAttributes(changes, 'testapp', 0, 0, name='Document')
self.assertMigrationDependencies(
changes, 'testapp', 0, [('__setting__', 'AUTH_USER_MODEL')],
)
def test_swappable_changed(self): def test_swappable_changed(self):
with isolate_lru_cache(apps.get_swappable_settings_name): with isolate_lru_cache(apps.get_swappable_settings_name):
before = self.make_project_state([self.custom_user, self.author_with_user]) before = self.make_project_state([self.custom_user, self.author_with_user])