From f04af7080b12744c8f06fb8228ef683d556690d0 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 12 Oct 2011 20:51:59 +0000 Subject: [PATCH] Introduce `ContentType.objects.get_for_models(*models)` and use it in the the auth permissions code. This is a solid performance gain on the test suite. Thanks to ptone for the profiling to find this hotspot, and carl for the review. git-svn-id: http://code.djangoproject.com/svn/django/trunk@16963 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/auth/management/__init__.py | 4 +- django/contrib/contenttypes/models.py | 61 +++++++++++++++++++--- django/contrib/contenttypes/tests.py | 30 +++++++++++ docs/ref/contrib/contenttypes.txt | 11 +++- 4 files changed, 96 insertions(+), 10 deletions(-) diff --git a/django/contrib/auth/management/__init__.py b/django/contrib/auth/management/__init__.py index 532654f92d..78a51cf443 100644 --- a/django/contrib/auth/management/__init__.py +++ b/django/contrib/auth/management/__init__.py @@ -31,8 +31,8 @@ def create_permissions(app, created_models, verbosity, **kwargs): searched_perms = list() # The codenames and ctypes that should exist. ctypes = set() - for klass in app_models: - ctype = ContentType.objects.get_for_model(klass) + ctypes_for_models = ContentType.objects.get_for_models(*app_models) + for klass, ctype in ctypes_for_models.iteritems(): ctypes.add(ctype) for perm in _get_all_permissions(klass._meta): searched_perms.append((ctype, perm)) diff --git a/django/contrib/contenttypes/models.py b/django/contrib/contenttypes/models.py index c7e3dd79af..1bb07e0d52 100644 --- a/django/contrib/contenttypes/models.py +++ b/django/contrib/contenttypes/models.py @@ -15,19 +15,26 @@ class ContentTypeManager(models.Manager): ct = self.get(app_label=app_label, model=model) return ct + def _get_opts(self, model): + opts = model._meta + while opts.proxy: + model = opts.proxy_for_model + opts = model._meta + return opts + + def _get_from_cache(self, opts): + key = (opts.app_label, opts.object_name.lower()) + return self.__class__._cache[self.db][key] + def get_for_model(self, model): """ Returns the ContentType object for a given model, creating the ContentType if necessary. Lookups are cached so that subsequent lookups for the same model don't hit the database. """ - opts = model._meta - while opts.proxy: - model = opts.proxy_for_model - opts = model._meta - key = (opts.app_label, opts.object_name.lower()) + opts = self._get_opts(model) try: - ct = self.__class__._cache[self.db][key] + ct = self._get_from_cache(opts) except KeyError: # Load or create the ContentType entry. The smart_unicode() is # needed around opts.verbose_name_raw because name_raw might be a @@ -41,6 +48,48 @@ class ContentTypeManager(models.Manager): return ct + def get_for_models(self, *models): + """ + Given *models, returns a dictionary mapping {model: content_type}. + """ + # Final results + results = {} + # models that aren't already in the cache + needed_app_labels = set() + needed_models = set() + needed_opts = set() + for model in models: + opts = self._get_opts(model) + try: + ct = self._get_from_cache(opts) + except KeyError: + needed_app_labels.add(opts.app_label) + needed_models.add(opts.object_name.lower()) + needed_opts.add(opts) + else: + results[model] = ct + if needed_opts: + cts = self.filter( + app_label__in=needed_app_labels, + model__in=needed_models + ) + for ct in cts: + model = ct.model_class() + if model._meta in needed_opts: + results[model] = ct + needed_opts.remove(model._meta) + self._add_to_cache(self.db, ct) + for opts in needed_opts: + # These weren't in the cache, or the DB, create them. + ct = self.create( + app_label=opts.app_label, + model=opts.object_name.lower(), + name=smart_unicode(opts.verbose_name_raw), + ) + self._add_to_cache(self.db, ct) + results[ct.model_class()] = ct + return results + def get_for_id(self, id): """ Lookup a ContentType by ID. Uses the same shared cache as get_for_model diff --git a/django/contrib/contenttypes/tests.py b/django/contrib/contenttypes/tests.py index e7f843ddd0..1854664755 100644 --- a/django/contrib/contenttypes/tests.py +++ b/django/contrib/contenttypes/tests.py @@ -63,6 +63,36 @@ class ContentTypesTests(TestCase): with self.assertNumQueries(1): ContentType.objects.get_for_model(ContentType) + def test_get_for_models_empty_cache(self): + # Empty cache. + with self.assertNumQueries(1): + cts = ContentType.objects.get_for_models(ContentType, FooWithUrl) + self.assertEqual(cts, { + ContentType: ContentType.objects.get_for_model(ContentType), + FooWithUrl: ContentType.objects.get_for_model(FooWithUrl), + }) + + def test_get_for_models_partial_cache(self): + # Partial cache + ContentType.objects.get_for_model(ContentType) + with self.assertNumQueries(1): + cts = ContentType.objects.get_for_models(ContentType, FooWithUrl) + self.assertEqual(cts, { + ContentType: ContentType.objects.get_for_model(ContentType), + FooWithUrl: ContentType.objects.get_for_model(FooWithUrl), + }) + + def test_get_for_models_full_cache(self): + # Full cache + ContentType.objects.get_for_model(ContentType) + ContentType.objects.get_for_model(FooWithUrl) + with self.assertNumQueries(0): + cts = ContentType.objects.get_for_models(ContentType, FooWithUrl) + self.assertEqual(cts, { + ContentType: ContentType.objects.get_for_model(ContentType), + FooWithUrl: ContentType.objects.get_for_model(FooWithUrl), + }) + def test_shortcut_view(self): """ Check that the shortcut view (used for the admin "view on site" diff --git a/docs/ref/contrib/contenttypes.txt b/docs/ref/contrib/contenttypes.txt index 914e06115b..fc685c8cd6 100644 --- a/docs/ref/contrib/contenttypes.txt +++ b/docs/ref/contrib/contenttypes.txt @@ -193,6 +193,13 @@ The ``ContentTypeManager`` :class:`~django.contrib.contenttypes.models.ContentType` instance representing that model. + .. method:: get_for_models(*models) + + Takes a variadic number of model classes, and returns a dictionary + mapping the model classes to the + :class:`~django.contrib.contenttypes.models.ContentType` instances + representing them. + .. method:: get_by_natural_key(app_label, model) Returns the :class:`~django.contrib.contenttypes.models.ContentType` @@ -319,8 +326,8 @@ creating a ``TaggedItem``:: Due to the way :class:`~django.contrib.contenttypes.generic.GenericForeignKey` is implemented, you cannot use such fields directly with filters (``filter()`` -and ``exclude()``, for example) via the database API. Because a -:class:`~django.contrib.contenttypes.generic.GenericForeignKey` isn't a +and ``exclude()``, for example) via the database API. Because a +:class:`~django.contrib.contenttypes.generic.GenericForeignKey` isn't a normal field objects, these examples will *not* work:: # This will fail