Replaced ad-hoc caching of get_models with lru_cache.

Invalidate properly the cache whenever all_models or app_configs change.
This fixes some isolation issues in the test suite.
This commit is contained in:
Aymeric Augustin 2013-12-24 10:08:04 +01:00
parent 2ec8e3443b
commit 82a35e24d4
8 changed files with 15 additions and 27 deletions

View File

@ -7,6 +7,7 @@ import warnings
from django.conf import settings from django.conf import settings
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.utils import lru_cache
from django.utils.module_loading import import_lock from django.utils.module_loading import import_lock
from django.utils._os import upath from django.utils._os import upath
@ -49,9 +50,6 @@ class AppCache(object):
# Pending lookups for lazy relations. # Pending lookups for lazy relations.
self._pending_lookups = {} self._pending_lookups = {}
# Cache for get_models.
self._get_models_cache = {}
def populate_apps(self, installed_apps=None): def populate_apps(self, installed_apps=None):
""" """
Populate app-related information. Populate app-related information.
@ -83,6 +81,7 @@ class AppCache(object):
app_config = AppConfig.create(app_name) app_config = AppConfig.create(app_name)
self.app_configs[app_config.label] = app_config self.app_configs[app_config.label] = app_config
self.get_models.cache_clear()
self._apps_loaded = True self._apps_loaded = True
def populate_models(self): def populate_models(self):
@ -131,6 +130,7 @@ class AppCache(object):
del self._postponed del self._postponed
self.get_models.cache_clear()
self._models_loaded = True self._models_loaded = True
def app_cache_ready(self): def app_cache_ready(self):
@ -180,6 +180,8 @@ class AppCache(object):
raise LookupError("App with label %r doesn't have a models module." % app_label) raise LookupError("App with label %r doesn't have a models module." % app_label)
return app_config return app_config
# This method is performance-critical at least for Django's test suite.
@lru_cache.lru_cache(maxsize=None)
def get_models(self, app_mod=None, def get_models(self, app_mod=None,
include_auto_created=False, include_deferred=False, include_auto_created=False, include_deferred=False,
only_installed=True, include_swapped=False): only_installed=True, include_swapped=False):
@ -206,12 +208,7 @@ class AppCache(object):
""" """
if not self.master: if not self.master:
only_installed = False only_installed = False
cache_key = (app_mod, include_auto_created, include_deferred, only_installed, include_swapped)
model_list = None model_list = None
try:
return self._get_models_cache[cache_key]
except KeyError:
pass
self.populate_models() self.populate_models()
if app_mod: if app_mod:
app_label = app_mod.__name__.split('.')[-2] app_label = app_mod.__name__.split('.')[-2]
@ -235,7 +232,6 @@ class AppCache(object):
(not model._meta.auto_created or include_auto_created) and (not model._meta.auto_created or include_auto_created) and
(not model._meta.swapped or include_swapped)) (not model._meta.swapped or include_swapped))
) )
self._get_models_cache[cache_key] = model_list
return model_list return model_list
def get_model(self, app_label, model_name, only_installed=True): def get_model(self, app_label, model_name, only_installed=True):
@ -272,7 +268,7 @@ class AppCache(object):
if os.path.splitext(fname1)[0] == os.path.splitext(fname2)[0]: if os.path.splitext(fname1)[0] == os.path.splitext(fname2)[0]:
return return
models[model_name] = model models[model_name] = model
self._get_models_cache.clear() self.get_models.cache_clear()
def has_app(self, app_name): def has_app(self, app_name):
""" """
@ -368,6 +364,7 @@ class AppCache(object):
app_config = AppConfig.create(app_name) app_config = AppConfig.create(app_name)
app_config.import_models(self.all_models[app_config.label]) app_config.import_models(self.all_models[app_config.label])
self.app_configs[app_config.label] = app_config self.app_configs[app_config.label] = app_config
self.get_models.cache_clear()
return app_config.models_module return app_config.models_module
def get_app(self, app_label): def get_app(self, app_label):

View File

@ -21,7 +21,7 @@ class EggLoadingTest(TestCase):
def tearDown(self): def tearDown(self):
app_cache.all_models['app_loading'] = self._old_models app_cache.all_models['app_loading'] = self._old_models
app_cache._get_models_cache = {} app_cache.get_models.cache_clear()
sys.path = self.old_path sys.path = self.old_path

View File

@ -128,7 +128,7 @@ class ManagersRegressionTests(TestCase):
finally: finally:
app_cache.app_configs['managers_regress'].models = _old_models app_cache.app_configs['managers_regress'].models = _old_models
app_cache.all_models['managers_regress'] = _old_models app_cache.all_models['managers_regress'] = _old_models
app_cache._get_models_cache = {} app_cache.get_models.cache_clear()
@override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent') @override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent')
def test_custom_swappable_manager(self): def test_custom_swappable_manager(self):
@ -156,7 +156,7 @@ class ManagersRegressionTests(TestCase):
finally: finally:
app_cache.app_configs['managers_regress'].models = _old_models app_cache.app_configs['managers_regress'].models = _old_models
app_cache.all_models['managers_regress'] = _old_models app_cache.all_models['managers_regress'] = _old_models
app_cache._get_models_cache = {} app_cache.get_models.cache_clear()
@override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent') @override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent')
def test_explicit_swappable_manager(self): def test_explicit_swappable_manager(self):
@ -184,7 +184,7 @@ class ManagersRegressionTests(TestCase):
finally: finally:
app_cache.app_configs['managers_regress'].models = _old_models app_cache.app_configs['managers_regress'].models = _old_models
app_cache.all_models['managers_regress'] = _old_models app_cache.all_models['managers_regress'] = _old_models
app_cache._get_models_cache = {} app_cache.get_models.cache_clear()
def test_regress_3871(self): def test_regress_3871(self):
related = RelatedModel.objects.create() related = RelatedModel.objects.create()

View File

@ -136,7 +136,7 @@ class MakeMigrationsTests(MigrationTestBase):
def tearDown(self): def tearDown(self):
app_cache.app_configs['migrations'].models = self._old_models app_cache.app_configs['migrations'].models = self._old_models
app_cache.all_models['migrations'] = self._old_models app_cache.all_models['migrations'] = self._old_models
app_cache._get_models_cache = {} app_cache.get_models.cache_clear()
os.chdir(self.test_dir) os.chdir(self.test_dir)
try: try:

View File

@ -175,7 +175,7 @@ class ProxyModelTests(TestCase):
finally: finally:
app_cache.app_configs['proxy_models'].models = _old_models app_cache.app_configs['proxy_models'].models = _old_models
app_cache.all_models['proxy_models'] = _old_models app_cache.all_models['proxy_models'] = _old_models
app_cache._get_models_cache = {} app_cache.get_models.cache_clear()
def test_myperson_manager(self): def test_myperson_manager(self):
Person.objects.create(name="fred") Person.objects.create(name="fred")

View File

@ -170,6 +170,7 @@ def setup(verbosity, test_labels):
app_config = AppConfig.create(module_label) app_config = AppConfig.create(module_label)
app_config.import_models(app_cache.all_models[app_config.label]) app_config.import_models(app_cache.all_models[app_config.label])
app_cache.app_configs[app_config.label] = app_config app_cache.app_configs[app_config.label] = app_config
app_cache.get_models.cache_clear()
return state return state

View File

@ -19,16 +19,6 @@ class SwappableModelTests(TestCase):
'django.contrib.contenttypes', 'django.contrib.contenttypes',
] ]
def setUp(self):
# This test modifies the installed apps, so we need to make sure
# we're not dealing with a cached app list.
app_cache._get_models_cache.clear()
def tearDown(self):
# By fiddling with swappable models, we alter the installed models
# cache, so flush it to make sure there are no side effects.
app_cache._get_models_cache.clear()
@override_settings(TEST_ARTICLE_MODEL='swappable_models.AlternateArticle') @override_settings(TEST_ARTICLE_MODEL='swappable_models.AlternateArticle')
def test_generated_data(self): def test_generated_data(self):
"Permissions and content types are not created for a swapped model" "Permissions and content types are not created for a swapped model"

View File

@ -37,7 +37,7 @@ class TablespacesTests(TestCase):
app_cache.app_configs['tablespaces'].models = self._old_models app_cache.app_configs['tablespaces'].models = self._old_models
app_cache.all_models['tablespaces'] = self._old_models app_cache.all_models['tablespaces'] = self._old_models
app_cache._get_models_cache = {} app_cache.get_models.cache_clear()
def assertNumContains(self, haystack, needle, count): def assertNumContains(self, haystack, needle, count):
real_count = haystack.count(needle) real_count = haystack.count(needle)