Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #35408 -- Optimized post-migrate permission creation. #18105

Merged
merged 1 commit into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 23 additions & 27 deletions django/contrib/auth/management/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ def create_permissions(
if not app_config.models_module:
return

try:
Permission = apps.get_model("auth", "Permission")
except LookupError:
return
if not router.allow_migrate_model(using, Permission):
return
Comment on lines +49 to +54
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved up here to early return before the extra call to create_contenttypes


# Ensure that contenttypes are created for this app. Needed if
# 'django.contrib.auth' is in INSTALLED_APPS before
# 'django.contrib.contenttypes'.
Expand All @@ -62,49 +69,38 @@ def create_permissions(
try:
app_config = apps.get_app_config(app_label)
ContentType = apps.get_model("contenttypes", "ContentType")
Permission = apps.get_model("auth", "Permission")
except LookupError:
return

if not router.allow_migrate_model(using, Permission):
return

# This will hold the permissions we're looking for as
# (content_type, (codename, name))
searched_perms = []
# The codenames and ctypes that should exist.
ctypes = set()
for klass in app_config.get_models():
# Force looking up the content types in the current database
# before creating foreign keys to them.
ctype = ContentType.objects.db_manager(using).get_for_model(
klass, for_concrete_model=False
)
models = list(app_config.get_models())

ctypes.add(ctype)
for perm in _get_all_permissions(klass._meta):
searched_perms.append((ctype, perm))
# Grab all the ContentTypes.
ctypes = ContentType.objects.db_manager(using).get_for_models(
*models, for_concrete_models=False
)

# Find all the Permissions that have a content_type for a model we're
# looking for. We don't need to check for codenames since we already have
# a list of the ones we're going to create.
all_perms = set(
Permission.objects.using(using)
.filter(
content_type__in=ctypes,
content_type__in=set(ctypes.values()),
)
.values_list("content_type", "codename")
)

perms = []
for ct, (codename, name) in searched_perms:
if (ct.pk, codename) not in all_perms:
permission = Permission()
permission._state.db = using
permission.codename = codename
permission.name = name
permission.content_type = ct
perms.append(permission)
for model in models:
ctype = ctypes[model]
for codename, name in _get_all_permissions(model._meta):
if (ctype.pk, codename) not in all_perms:
permission = Permission()
permission._state.db = using
permission.codename = codename
permission.name = name
permission.content_type = ctype
perms.append(permission)

Permission.objects.using(using).bulk_create(perms)
if verbosity >= 2:
Expand Down
2 changes: 1 addition & 1 deletion tests/auth_tests/test_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -1528,7 +1528,7 @@ class CreatePermissionsMultipleDatabasesTests(TestCase):

def test_set_permissions_fk_to_using_parameter(self):
Permission.objects.using("other").delete()
with self.assertNumQueries(6, using="other") as captured_queries:
with self.assertNumQueries(4, using="other") as captured_queries:
create_permissions(apps.get_app_config("auth"), verbosity=0, using="other")
self.assertIn("INSERT INTO", captured_queries[-1]["sql"].upper())
self.assertGreater(Permission.objects.using("other").count(), 0)