From 859a87d873ce7152af73ab851653b4e1c3ffea4c Mon Sep 17 00:00:00 2001 From: Biel Frontera Date: Mon, 14 Mar 2022 09:53:36 +0100 Subject: [PATCH] Fixed #31357 -- Fixed get_for_models() crash for stale content types when model with the same name exists in another app. --- django/contrib/contenttypes/models.py | 18 ++++++++++++------ tests/contenttypes_tests/test_models.py | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/django/contrib/contenttypes/models.py b/django/contrib/contenttypes/models.py index ef4f30556f..b83c9afcae 100644 --- a/django/contrib/contenttypes/models.py +++ b/django/contrib/contenttypes/models.py @@ -2,6 +2,7 @@ from collections import defaultdict from django.apps import apps from django.db import models +from django.db.models import Q from django.utils.translation import gettext_lazy as _ @@ -64,9 +65,8 @@ class ContentTypeManager(models.Manager): Given *models, return a dictionary mapping {model: content_type}. """ results = {} - # Models that aren't already in the cache. - needed_app_labels = set() - needed_models = set() + # Models that aren't already in the cache grouped by app labels. + needed_models = defaultdict(set) # Mapping of opts to the list of models requiring it. needed_opts = defaultdict(list) for model in models: @@ -74,14 +74,20 @@ class ContentTypeManager(models.Manager): try: ct = self._get_from_cache(opts) except KeyError: - needed_app_labels.add(opts.app_label) - needed_models.add(opts.model_name) + needed_models[opts.app_label].add(opts.model_name) needed_opts[opts].append(model) else: results[model] = ct if needed_opts: # Lookup required content types from the DB. - cts = self.filter(app_label__in=needed_app_labels, model__in=needed_models) + condition = Q( + *( + Q(("app_label", app_label), ("model__in", models), _connector=Q.AND) + for app_label, models in needed_models.items() + ), + _connector=Q.OR, + ) + cts = self.filter(condition) for ct in cts: opts_models = needed_opts.pop(ct.model_class()._meta, []) for model in opts_models: diff --git a/tests/contenttypes_tests/test_models.py b/tests/contenttypes_tests/test_models.py index e2fb7fe818..a96e12e69f 100644 --- a/tests/contenttypes_tests/test_models.py +++ b/tests/contenttypes_tests/test_models.py @@ -248,6 +248,26 @@ class ContentTypesTests(TestCase): ct_fetched = ContentType.objects.get_for_id(ct.pk) self.assertIsNone(ct_fetched.model_class()) + def test_missing_model_with_existing_model_name(self): + """ + Displaying content types in admin (or anywhere) doesn't break on + leftover content type records in the DB for which no model is defined + anymore, even if a model with the same name exists in another app. + """ + # Create a stale ContentType that matches the name of an existing + # model. + ContentType.objects.create(app_label="contenttypes", model="author") + ContentType.objects.clear_cache() + # get_for_models() should work as expected for existing models. + cts = ContentType.objects.get_for_models(ContentType, Author) + self.assertEqual( + cts, + { + ContentType: ContentType.objects.get_for_model(ContentType), + Author: ContentType.objects.get_for_model(Author), + }, + ) + def test_str(self): ct = ContentType.objects.get(app_label="contenttypes_tests", model="site") self.assertEqual(str(ct), "contenttypes_tests | site")