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

@@ -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. <module 'django.contrib.admin' from
         # 'django/contrib/admin/__init__.pyc'>.
-        self.app_module = app_module
+        self.app_module = import_module(app_name)
 
         # Module containing models eg. <module 'django.contrib.admin.models'
-        # from 'django/contrib/admin/models.pyc'>. 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 '<AppConfig: %s>' % 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)

django/core/apps/cache.py

@@ -12,10 +12,7 @@
 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,13 +51,93 @@ def __init__(self, master=False):
         # 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()
         self.postponed = []
         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 @@ def load_app(self, app_name, can_postpone=False):
         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 @@ def unset_available_apps(self):
 
     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
 

tests/test_runner/tests.py

@@ -3,7 +3,6 @@
 """
 from __future__ import unicode_literals
 
-from importlib import import_module
 from optparse import make_option
 import types
 import unittest
@@ -227,22 +226,19 @@ def test_get_tests(self):
         "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)
 
     def test_import_error(self):
         "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):