Refactored registration of models.

Got rid of AppConfig._stub. As a side effect, app_cache.app_configs now
only contains entries for applications that are in INSTALLED_APPS, which
is a good thing and will allow dramatic simplifications (which I will
perform in the next commit). That required adjusting all methods that
iterate on app_configs without checking the "installed" flag, hence the
large changes in get_model[s].

Introduced AppCache.all_models to store models:
- while the app cache is being populated and a suitable app config
  object to register models isn't available yet;
- for applications that aren't in INSTALLED_APPS since they don't have
  an app config any longer.

Replaced get_model(seed_cache=False) by registered_model() which can be
kept simple and safe to call at any time, and removed the seed_cache
argument to get_model[s]. There's no replacement for that private API.

Allowed non-master app caches to go through populate() as it is now
safe to do so. They were introduced in 1.7 so backwards compatibility
isn't a concern as long as the migrations framework keeps working.

django/core/apps/base.py

@@ -39,9 +39,5 @@ class AppConfig(object):
         # This is a unicode object on Python 2 and a str on Python 3.
         self.path = upath(app_module.__path__[0]) if app_module is not None else None
 
-    @classmethod
-    def _stub(cls, label):
-        return cls(label, None, None)
-
     def __repr__(self):
         return '<AppConfig: %s>' % self.label

django/core/apps/cache.py

@@ -1,6 +1,6 @@
 "Utilities for loading models and the modules that contain them."
 
-from collections import OrderedDict
+from collections import defaultdict, OrderedDict
 from importlib import import_module
 import os
 import sys
@@ -10,7 +10,6 @@ from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.utils.module_loading import import_lock, module_has_submodule
 from django.utils._os import upath
-from django.utils import six
 
 from .base import AppConfig
 
@@ -39,6 +38,11 @@ class AppCache(object):
         # get_model[s].
         self.master = master
 
+        # Mapping of app labels => model names => model classes. Used to
+        # register models before the app cache is populated and also for
+        # applications that aren't installed.
+        self.all_models = defaultdict(OrderedDict)
+
         # Mapping of labels to AppConfig instances for installed apps.
         self.app_configs = OrderedDict()
 
@@ -118,10 +122,7 @@ class AppCache(object):
 
         app_config = AppConfig(
             name=app_name, app_module=app_module, models_module=models_module)
-        # If a stub config existed for this app, preserve models registry.
+        app_config.models = self.all_models[app_config.label]
-        old_app_config = self.app_configs.get(app_config.label)
-        if old_app_config is not None:
-            app_config.models = old_app_config.models
         self.app_configs[app_config.label] = app_config
 
         return models_module
@@ -145,6 +146,8 @@ class AppCache(object):
         If only_with_models_module in True (non-default), only applications
         containing a models module are considered.
         """
+        if not only_installed:
+            raise ValueError("only_installed=False isn't supported any more.")
         self.populate()
         for app_config in self.app_configs.values():
             if only_installed and not app_config.installed:
@@ -170,6 +173,8 @@ class AppCache(object):
         If only_with_models_module in True (non-default), only applications
         containing a models module are considered.
         """
+        if not only_installed:
+            raise ValueError("only_installed=False isn't supported any more.")
         self.populate()
         app_config = self.app_configs.get(app_label)
         if app_config is None:
@@ -223,20 +228,22 @@ class AppCache(object):
         self.populate()
         if app_mod:
             app_label = app_mod.__name__.split('.')[-2]
-            try:
-                app_config = self.app_configs[app_label]
-            except KeyError:
-                app_list = []
-            else:
-                app_list = [app_config] if app_config.installed else []
-        else:
-            app_list = six.itervalues(self.app_configs)
             if only_installed:
-                app_list = (app for app in app_list if app.installed)
+                try:
+                    model_dicts = [self.app_configs[app_label].models]
+                except KeyError:
+                    model_dicts = []
+            else:
+                model_dicts = [self.all_models[app_label]]
+        else:
+            if only_installed:
+                model_dicts = [app_config.models for app_config in self.app_configs.values()]
+            else:
+                model_dicts = self.all_models.values()
         model_list = []
-        for app in app_list:
+        for model_dict in model_dicts:
             model_list.extend(
-                model for model in app.models.values()
+                model for model in model_dict.values()
                 if ((not model._deferred or include_deferred) and
                     (not model._meta.auto_created or include_auto_created) and
                     (not model._meta.swapped or include_swapped))
@@ -249,8 +256,7 @@ class AppCache(object):
             ]
         return model_list
 
-    def get_model(self, app_label, model_name,
+    def get_model(self, app_label, model_name, only_installed=True):
-                  seed_cache=True, only_installed=True):
         """
         Returns the model matching the given app_label and case-insensitive
         model_name.
@@ -262,43 +268,45 @@ class AppCache(object):
         """
         if not self.master:
             only_installed = False
-        if seed_cache:
+        self.populate()
-            self.populate()
         if only_installed:
             app_config = self.app_configs.get(app_label)
-            if app_config is not None and not app_config.installed:
+            if app_config is None or not app_config.installed:
                 return None
             if (self.available_apps is not None
                     and app_config.name not in self.available_apps):
                 raise UnavailableApp("App with label %s isn't available." % app_label)
-        try:
+        return self.all_models[app_label].get(model_name.lower())
-            return self.app_configs[app_label].models[model_name.lower()]
-        except KeyError:
-            return None
 
     def register_model(self, app_label, model):
-        try:
+        # Since this method is called when models are imported, it cannot
-            app_config = self.app_configs[app_label]
+        # perform imports because of the risk of import loops. It mustn't
-        except KeyError:
+        # call get_app_config().
-            app_config = AppConfig._stub(app_label)
-            self.app_configs[app_label] = app_config
-        # Add the model to the app_config's models dictionary.
         model_name = model._meta.model_name
-        model_dict = app_config.models
+        models = self.all_models[app_label]
-        if model_name in model_dict:
+        if model_name in models:
             # The same model may be imported via different paths (e.g.
             # appname.models and project.appname.models). We use the source
             # filename as a means to detect identity.
             fname1 = os.path.abspath(upath(sys.modules[model.__module__].__file__))
-            fname2 = os.path.abspath(upath(sys.modules[model_dict[model_name].__module__].__file__))
+            fname2 = os.path.abspath(upath(sys.modules[models[model_name].__module__].__file__))
             # Since the filename extension could be .py the first time and
             # .pyc or .pyo the second time, ignore the extension when
             # comparing.
             if os.path.splitext(fname1)[0] == os.path.splitext(fname2)[0]:
                 return
-        model_dict[model_name] = model
+        models[model_name] = model
         self._get_models_cache.clear()
 
+    def registered_model(self, app_label, model_name):
+        """
+        Test if a model is registered and return the model class or None.
+
+        It's safe to call this method at import time, even while the app cache
+        is being populated.
+        """
+        return self.all_models[app_label].get(model_name.lower())
+
     def set_available_apps(self, available):
         available = set(available)
         installed = set(settings.INSTALLED_APPS)

django/db/models/base.py

@@ -151,8 +151,7 @@ class ModelBase(type):
                 new_class._base_manager = new_class._base_manager._copy_to_model(new_class)
 
         # Bail out early if we have already created this class.
-        m = new_class._meta.app_cache.get_model(new_class._meta.app_label, name,
+        m = new_class._meta.app_cache.registered_model(new_class._meta.app_label, name)
-                      seed_cache=False, only_installed=False)
         if m is not None:
             return m
 
@@ -279,8 +278,7 @@ class ModelBase(type):
         # the first time this model tries to register with the framework. There
         # should only be one class for each model, so we always return the
         # registered version.
-        return new_class._meta.app_cache.get_model(new_class._meta.app_label, name,
+        return new_class._meta.app_cache.registered_model(new_class._meta.app_label, name)
-                         seed_cache=False, only_installed=False)
 
     def copy_managers(cls, base_managers):
         # This is in-place sorting of an Options attribute, but that's fine.

django/db/models/fields/related.py

@@ -68,8 +68,7 @@ def add_lazy_relation(cls, field, relation, operation):
     # string right away. If get_model returns None, it means that the related
     # model isn't loaded yet, so we need to pend the relation until the class
     # is prepared.
-    model = cls._meta.app_cache.get_model(app_label, model_name,
+    model = cls._meta.app_cache.registered_model(app_label, model_name)
-                      seed_cache=False, only_installed=False)
     if model:
         operation(field, model, cls)
     else:

tests/app_loading/tests.py

@@ -22,6 +22,7 @@ class EggLoadingTest(TestCase):
 
     def tearDown(self):
         app_cache.app_configs['app_loading'].models = self._old_models
+        app_cache.all_models['app_loading'] = self._old_models
         app_cache._get_models_cache = {}
 
         sys.path = self.old_path
@@ -80,9 +81,9 @@ class EggLoadingTest(TestCase):
         app_cache.master = True
         with override_settings(INSTALLED_APPS=('notexists',)):
             with self.assertRaises(ImportError):
-                app_cache.get_model('notexists', 'nomodel', seed_cache=True)
+                app_cache.get_model('notexists', 'nomodel')
             with self.assertRaises(ImportError):
-                app_cache.get_model('notexists', 'nomodel', seed_cache=True)
+                app_cache.get_model('notexists', 'nomodel')
 
 
 class GetModelsTest(TestCase):
@@ -101,17 +102,17 @@ class GetModelsTest(TestCase):
             self.not_installed_module.NotInstalledModel)
 
     def test_get_models_only_returns_installed_models(self):
-        self.assertFalse(
+        self.assertNotIn(
-            "NotInstalledModel" in
+            "NotInstalledModel",
             [m.__name__ for m in app_cache.get_models()])
 
     def test_get_models_with_app_label_only_returns_installed_models(self):
         self.assertEqual(app_cache.get_models(self.not_installed_module), [])
 
     def test_get_models_with_not_installed(self):
-        self.assertTrue(
+        self.assertIn(
-            "NotInstalledModel" in [
+            "NotInstalledModel",
-                m.__name__ for m in app_cache.get_models(only_installed=False)])
+            [m.__name__ for m in app_cache.get_models(only_installed=False)])
 
 
 class NotInstalledModelsTest(TestCase):

tests/invalid_models/tests.py

@@ -24,6 +24,7 @@ class InvalidModelTestCase(unittest.TestCase):
 
     def tearDown(self):
         app_cache.app_configs['invalid_models'].models = self._old_models
+        app_cache.all_models['invalid_models'] = self._old_models
         app_cache._get_models_cache = {}
         sys.stdout = self.old_stdout
 

tests/managers_regress/tests.py

@@ -128,6 +128,7 @@ class ManagersRegressionTests(TestCase):
 
         finally:
             app_cache.app_configs['managers_regress'].models = _old_models
+            app_cache.all_models['managers_regress'] = _old_models
             app_cache._get_models_cache = {}
 
     @override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent')
@@ -155,6 +156,7 @@ class ManagersRegressionTests(TestCase):
 
         finally:
             app_cache.app_configs['managers_regress'].models = _old_models
+            app_cache.all_models['managers_regress'] = _old_models
             app_cache._get_models_cache = {}
 
     @override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent')
@@ -182,6 +184,7 @@ class ManagersRegressionTests(TestCase):
 
         finally:
             app_cache.app_configs['managers_regress'].models = _old_models
+            app_cache.all_models['managers_regress'] = _old_models
             app_cache._get_models_cache = {}
 
     def test_regress_3871(self):

tests/migrations/test_commands.py

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

tests/proxy_model_inheritance/tests.py

@@ -34,6 +34,8 @@ class ProxyModelInheritanceTests(TransactionTestCase):
         sys.path = self.old_sys_path
         del app_cache.app_configs['app1']
         del app_cache.app_configs['app2']
+        del app_cache.all_models['app1']
+        del app_cache.all_models['app2']
 
     def test_table_exists(self):
         try:

tests/proxy_models/tests.py

@@ -175,6 +175,7 @@ class ProxyModelTests(TestCase):
                         proxy = True
         finally:
             app_cache.app_configs['proxy_models'].models = _old_models
+            app_cache.all_models['proxy_models'] = _old_models
             app_cache._get_models_cache = {}
 
     def test_myperson_manager(self):

tests/tablespaces/tests.py

@@ -36,6 +36,7 @@ class TablespacesTests(TestCase):
             model._meta.managed = False
 
         app_cache.app_configs['tablespaces'].models = self._old_models
+        app_cache.all_models['tablespaces'] = self._old_models
         app_cache._get_models_cache = {}
 
     def assertNumContains(self, haystack, needle, count):