From 2b56d6910269ace19e3007d2b4a3445f7886314f Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Wed, 18 Dec 2013 11:28:21 +0100 Subject: [PATCH] Implemented two-stage app-cache population. First stage imports app modules. It doesn't catch import errors. This matches the previous behavior and keeps the code simple. Second stage import models modules. It catches import errors and retries them after walking through the entire list once. This matches the previous behavior and seems useful. populate_models() is intended to be equivalent to populate(). It isn't wired yet. That is coming in the next commit. --- django/core/apps/base.py | 38 ++++++++++----- django/core/apps/cache.py | 95 ++++++++++++++++++++++++++++++++++---- tests/test_runner/tests.py | 16 +++---- 3 files changed, 117 insertions(+), 32 deletions(-) diff --git a/django/core/apps/base.py b/django/core/apps/base.py index 332a066bcb5..83c073a12ad 100644 --- a/django/core/apps/base.py +++ b/django/core/apps/base.py @@ -1,39 +1,53 @@ -from collections import OrderedDict +from importlib import import_module +from django.utils.module_loading import module_has_submodule from django.utils._os import upath +MODELS_MODULE_NAME = 'models' + + class AppConfig(object): """ Class representing a Django application and its configuration. """ - def __init__(self, name, app_module, models_module): + def __init__(self, app_name): # Full Python path to the application eg. 'django.contrib.admin'. # This is the value that appears in INSTALLED_APPS. - self.name = name + self.name = app_name # Last component of the Python path to the application eg. 'admin'. # This value must be unique across a Django project. - self.label = name.rpartition(".")[2] + self.label = app_name.rpartition(".")[2] # Root module eg. . - self.app_module = app_module + self.app_module = import_module(app_name) # Module containing models eg. . None if the application - # doesn't have a models module. - self.models_module = models_module + # from 'django/contrib/admin/models.pyc'>. Set by import_models(). + # None if the application doesn't have a models module. + self.models_module = None - # Mapping of lower case model names to model classes. - # Populated by calls to AppCache.register_model(). - self.models = OrderedDict() + # Mapping of lower case model names to model classes. Initally set to + # None to prevent accidental access before import_models() runs. + self.models = None # Filesystem path to the application directory eg. # u'/usr/lib/python2.7/dist-packages/django/contrib/admin'. # This is a unicode object on Python 2 and a str on Python 3. - self.path = upath(app_module.__path__[0]) + self.path = upath(self.app_module.__path__[0]) def __repr__(self): return '' % self.label + + def import_models(self, all_models): + # Dictionary of models for this app, stored in the 'all_models' + # attribute of the AppCache this AppConfig is attached to. Injected as + # a parameter because it may get populated before this method has run. + self.models = all_models + + if module_has_submodule(self.app_module, MODELS_MODULE_NAME): + models_module_name = '%s.%s' % (self.name, MODELS_MODULE_NAME) + self.models_module = import_module(models_module_name) diff --git a/django/core/apps/cache.py b/django/core/apps/cache.py index fb7bb6172e9..cf2489185ea 100644 --- a/django/core/apps/cache.py +++ b/django/core/apps/cache.py @@ -12,10 +12,7 @@ from django.core.exceptions import ImproperlyConfigured from django.utils.module_loading import import_lock, module_has_submodule from django.utils._os import upath -from .base import AppConfig - - -MODELS_MODULE_NAME = 'models' +from .base import AppConfig, MODELS_MODULE_NAME class UnavailableApp(Exception): @@ -54,6 +51,10 @@ class AppCache(object): # Used by TransactionTestCase.available_apps for performance reasons. self.available_apps = None + # Internal flags used when populating the cache. + self._apps_loaded = False + self._models_loaded = False + # -- Everything below here is only used when populating the cache -- self.loaded = False self.handled = set() @@ -61,6 +62,82 @@ class AppCache(object): self.nesting_level = 0 self._get_models_cache = {} + def populate_apps(self): + """ + Populate app-related information. + + This method imports each application module. + + It is thread safe and idempotent, but not reentrant. + """ + if self._apps_loaded: + return + # Since populate_apps() may be a side effect of imports, and since + # it will itself import modules, an ABBA deadlock between threads + # would be possible if we didn't take the import lock. See #18251. + with import_lock(): + if self._apps_loaded: + return + + # app_config should be pristine, otherwise the code below won't + # guarantee that the order matches the order in INSTALLED_APPS. + if self.app_configs: + raise RuntimeError("populate_apps() isn't reentrant") + + # Application modules aren't expected to import anything, and + # especially not other application modules, even indirectly. + # Therefore we simply import them sequentially. + for app_name in settings.INSTALLED_APPS: + app_config = AppConfig(app_name) + self.app_configs[app_config.label] = app_config + + self._apps_loaded = True + + def populate_models(self): + """ + Populate model-related information. + + This method imports each models module. + + It is thread safe, idempotent and reentrant. + """ + if self._models_loaded: + return + # Since populate_models() may be a side effect of imports, and since + # it will itself import modules, an ABBA deadlock between threads + # would be possible if we didn't take the import lock. See #18251. + with import_lock(): + if self._models_loaded: + return + + self.populate_apps() + + # Models modules are likely to import other models modules, for + # example to reference related objects. As a consequence: + # - we deal with import loops by postponing affected modules. + # - we provide reentrancy by making import_models() idempotent. + + outermost = not hasattr(self, '_postponed') + if outermost: + self._postponed = [] + + for app_config in self.app_configs.values(): + + try: + all_models = self.all_models[app_config.label] + app_config.import_models(all_models) + except ImportError: + self._postponed.append(app_config) + + if outermost: + for app_config in self._postponed: + all_models = self.all_models[app_config.label] + app_config.import_models(all_models) + + del self._postponed + + self._models_loaded = True + def populate(self): """ Fill in all the cache information. This method is threadsafe, in the @@ -121,8 +198,8 @@ class AppCache(object): finally: self.nesting_level -= 1 - app_config = AppConfig(app_name, app_module, models_module) - app_config.models = self.all_models[app_config.label] + app_config = AppConfig(app_name) + app_config.import_models(self.all_models[app_config.label]) self.app_configs[app_config.label] = app_config return models_module @@ -308,13 +385,11 @@ class AppCache(object): def _begin_with_app(self, app_name): # Returns an opaque value that can be passed to _end_with_app(). - app_module = import_module(app_name) - models_module = import_module('%s.models' % app_name) - app_config = AppConfig(app_name, app_module, models_module) + app_config = AppConfig(app_name) if app_config.label in self.app_configs: return None else: - app_config.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 return app_config diff --git a/tests/test_runner/tests.py b/tests/test_runner/tests.py index 04fa3b8784d..a6dbeb0ae4e 100644 --- a/tests/test_runner/tests.py +++ b/tests/test_runner/tests.py @@ -3,7 +3,6 @@ Tests for django test runner """ from __future__ import unicode_literals -from importlib import import_module from optparse import make_option import types import unittest @@ -227,10 +226,8 @@ class ModulesTestsPackages(IgnoreAllDeprecationWarningsMixin, unittest.TestCase) "Check that the get_tests helper function can find tests in a directory" from django.core.apps.base import AppConfig from django.test.simple import get_tests - app_config = AppConfig( - 'test_runner.valid_app', - import_module('test_runner.valid_app'), - import_module('test_runner.valid_app.models')) + app_config = AppConfig('test_runner.valid_app') + app_config.import_models({}) tests = get_tests(app_config) self.assertIsInstance(tests, types.ModuleType) @@ -238,11 +235,10 @@ class ModulesTestsPackages(IgnoreAllDeprecationWarningsMixin, unittest.TestCase) "Test for #12658 - Tests with ImportError's shouldn't fail silently" from django.core.apps.base import AppConfig from django.test.simple import get_tests - app_config = AppConfig( - 'test_runner_invalid_app', - import_module('test_runner_invalid_app'), - import_module('test_runner_invalid_app.models')) - self.assertRaises(ImportError, get_tests, app_config) + app_config = AppConfig('test_runner_invalid_app') + app_config.import_models({}) + with self.assertRaises(ImportError): + get_tests(app_config) class Sqlite3InMemoryTestDbs(TestCase):