Fixed #26286 -- Prevented content type managers from sharing their cache.

This should prevent managers methods from returning content type instances
registered to foreign apps now that these managers are also attached to models
created during migration phases.

Thanks Tim for the review.

Refs #23822.
This commit is contained in:
Simon Charette 2015-12-18 14:49:23 -05:00
parent b9519b2730
commit 3938b3ccaa
4 changed files with 30 additions and 10 deletions

View File

@ -10,13 +10,15 @@ from django.utils.translation import ugettext_lazy as _
class ContentTypeManager(models.Manager): class ContentTypeManager(models.Manager):
use_in_migrations = True use_in_migrations = True
# Cache to avoid re-looking up ContentType objects all over the place. def __init__(self, *args, **kwargs):
# This cache is shared by all the get_for_* methods. super(ContentTypeManager, self).__init__(*args, **kwargs)
_cache = {} # Cache shared by all the get_for_* methods to speed up
# ContentType retrieval.
self._cache = {}
def get_by_natural_key(self, app_label, model): def get_by_natural_key(self, app_label, model):
try: try:
ct = self.__class__._cache[self.db][(app_label, model)] ct = self._cache[self.db][(app_label, model)]
except KeyError: except KeyError:
ct = self.get(app_label=app_label, model=model) ct = self.get(app_label=app_label, model=model)
self._add_to_cache(self.db, ct) self._add_to_cache(self.db, ct)
@ -31,7 +33,7 @@ class ContentTypeManager(models.Manager):
def _get_from_cache(self, opts): def _get_from_cache(self, opts):
key = (opts.app_label, opts.model_name) key = (opts.app_label, opts.model_name)
return self.__class__._cache[self.db][key] return self._cache[self.db][key]
def get_for_model(self, model, for_concrete_model=True): def get_for_model(self, model, for_concrete_model=True):
""" """
@ -118,7 +120,7 @@ class ContentTypeManager(models.Manager):
(though ContentTypes are obviously not created on-the-fly by get_by_id). (though ContentTypes are obviously not created on-the-fly by get_by_id).
""" """
try: try:
ct = self.__class__._cache[self.db][id] ct = self._cache[self.db][id]
except KeyError: except KeyError:
# This could raise a DoesNotExist; that's correct behavior and will # This could raise a DoesNotExist; that's correct behavior and will
# make sure that only correct ctypes get stored in the cache dict. # make sure that only correct ctypes get stored in the cache dict.
@ -133,15 +135,15 @@ class ContentTypeManager(models.Manager):
django.contrib.contenttypes.management.update_contenttypes for where django.contrib.contenttypes.management.update_contenttypes for where
this gets called). this gets called).
""" """
self.__class__._cache.clear() self._cache.clear()
def _add_to_cache(self, using, ct): def _add_to_cache(self, using, ct):
"""Insert a ContentType into the cache.""" """Insert a ContentType into the cache."""
# Note it's possible for ContentType objects to be stale; model_class() will return None. # Note it's possible for ContentType objects to be stale; model_class() will return None.
# Hence, there is no reliance on model._meta.app_label here, just using the model fields instead. # Hence, there is no reliance on model._meta.app_label here, just using the model fields instead.
key = (ct.app_label, ct.model) key = (ct.app_label, ct.model)
self.__class__._cache.setdefault(using, {})[key] = ct self._cache.setdefault(using, {})[key] = ct
self.__class__._cache.setdefault(using, {})[ct.id] = ct self._cache.setdefault(using, {})[ct.id] = ct
@python_2_unicode_compatible @python_2_unicode_compatible

View File

@ -26,3 +26,6 @@ Bugfixes
``URLValidator`` to fix a regression in Django 1.8 (:ticket:`26204`). ``URLValidator`` to fix a regression in Django 1.8 (:ticket:`26204`).
* Fixed ``BoundField`` to reallow slices of subwidgets (:ticket:`26267`). * Fixed ``BoundField`` to reallow slices of subwidgets (:ticket:`26267`).
* Prevented ``ContentTypeManager`` instances from sharing their cache
(:ticket:`26286`).

View File

@ -49,3 +49,6 @@ Bugfixes
* Fixed a crash when passing a nonexistent template name to the cached template * Fixed a crash when passing a nonexistent template name to the cached template
loader's ``load_template()`` method (:ticket:`26280`). loader's ``load_template()`` method (:ticket:`26280`).
* Prevented ``ContentTypeManager`` instances from sharing their cache
(:ticket:`26286`).

View File

@ -1,6 +1,6 @@
from __future__ import unicode_literals from __future__ import unicode_literals
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType, ContentTypeManager
from django.contrib.contenttypes.views import shortcut from django.contrib.contenttypes.views import shortcut
from django.contrib.sites.shortcuts import get_current_site from django.contrib.sites.shortcuts import get_current_site
from django.db.utils import IntegrityError, OperationalError, ProgrammingError from django.db.utils import IntegrityError, OperationalError, ProgrammingError
@ -168,6 +168,18 @@ class ContentTypesTests(TestCase):
DeferredProxyModel: proxy_model_ct, DeferredProxyModel: proxy_model_ct,
}) })
def test_cache_not_shared_between_managers(self):
with self.assertNumQueries(1):
ContentType.objects.get_for_model(ContentType)
with self.assertNumQueries(0):
ContentType.objects.get_for_model(ContentType)
other_manager = ContentTypeManager()
other_manager.model = ContentType
with self.assertNumQueries(1):
other_manager.get_for_model(ContentType)
with self.assertNumQueries(0):
other_manager.get_for_model(ContentType)
@override_settings(ALLOWED_HOSTS=['example.com']) @override_settings(ALLOWED_HOSTS=['example.com'])
def test_shortcut_view(self): def test_shortcut_view(self):
""" """