summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAymeric Augustin <aymeric.augustin@m4x.org>2013-12-18 11:19:56 +0100
committerAymeric Augustin <aymeric.augustin@m4x.org>2013-12-22 11:39:17 +0100
commit742ed9878e7edbb7a11667c489c719c4d9ab82de (patch)
treed7a39020d0a9648f33911c92bab12928a3b6fb48
parent73c9e65b75cd956410ae71f39fb2f6a9b7b45b42 (diff)
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.
-rw-r--r--django/core/apps/base.py4
-rw-r--r--django/core/apps/cache.py76
-rw-r--r--django/db/models/base.py6
-rw-r--r--django/db/models/fields/related.py3
-rw-r--r--tests/app_loading/tests.py15
-rw-r--r--tests/invalid_models/tests.py1
-rw-r--r--tests/managers_regress/tests.py3
-rw-r--r--tests/migrations/test_commands.py1
-rw-r--r--tests/proxy_model_inheritance/tests.py2
-rw-r--r--tests/proxy_models/tests.py1
-rw-r--r--tests/tablespaces/tests.py1
11 files changed, 62 insertions, 51 deletions
diff --git a/django/core/apps/base.py b/django/core/apps/base.py
index 860777bb03..5991029c09 100644
--- a/django/core/apps/base.py
+++ b/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
diff --git a/django/core/apps/cache.py b/django/core/apps/cache.py
index d07c3bbe34..deffc84586 100644
--- a/django/core/apps/cache.py
+++ b/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.
- old_app_config = self.app_configs.get(app_config.label)
- if old_app_config is not None:
- app_config.models = old_app_config.models
+ app_config.models = self.all_models[app_config.label]
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 = []
+ if only_installed:
+ try:
+ model_dicts = [self.app_configs[app_label].models]
+ except KeyError:
+ model_dicts = []
else:
- app_list = [app_config] if app_config.installed else []
+ model_dicts = [self.all_models[app_label]]
else:
- app_list = six.itervalues(self.app_configs)
if only_installed:
- app_list = (app for app in app_list if app.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,
- seed_cache=True, only_installed=True):
+ def get_model(self, app_label, model_name, 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.app_configs[app_label].models[model_name.lower()]
- except KeyError:
- return None
+ return self.all_models[app_label].get(model_name.lower())
def register_model(self, app_label, model):
- try:
- app_config = self.app_configs[app_label]
- except KeyError:
- app_config = AppConfig._stub(app_label)
- self.app_configs[app_label] = app_config
- # Add the model to the app_config's models dictionary.
+ # Since this method is called when models are imported, it cannot
+ # perform imports because of the risk of import loops. It mustn't
+ # call get_app_config().
model_name = model._meta.model_name
- model_dict = app_config.models
- if model_name in model_dict:
+ models = self.all_models[app_label]
+ 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)
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 56973d4e38..a694350b97 100644
--- a/django/db/models/base.py
+++ b/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,
- seed_cache=False, only_installed=False)
+ m = new_class._meta.app_cache.registered_model(new_class._meta.app_label, name)
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,
- seed_cache=False, only_installed=False)
+ return new_class._meta.app_cache.registered_model(new_class._meta.app_label, name)
def copy_managers(cls, base_managers):
# This is in-place sorting of an Options attribute, but that's fine.
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index 3eb6799a5f..1b141af099 100644
--- a/django/db/models/fields/related.py
+++ b/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,
- seed_cache=False, only_installed=False)
+ model = cls._meta.app_cache.registered_model(app_label, model_name)
if model:
operation(field, model, cls)
else:
diff --git a/tests/app_loading/tests.py b/tests/app_loading/tests.py
index b32f510a24..e282306ec0 100644
--- a/tests/app_loading/tests.py
+++ b/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(
- "NotInstalledModel" in
+ self.assertNotIn(
+ "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(
- "NotInstalledModel" in [
- m.__name__ for m in app_cache.get_models(only_installed=False)])
+ self.assertIn(
+ "NotInstalledModel",
+ [m.__name__ for m in app_cache.get_models(only_installed=False)])
class NotInstalledModelsTest(TestCase):
diff --git a/tests/invalid_models/tests.py b/tests/invalid_models/tests.py
index 712484c611..10d8fa47a7 100644
--- a/tests/invalid_models/tests.py
+++ b/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
diff --git a/tests/managers_regress/tests.py b/tests/managers_regress/tests.py
index f2897862ec..b9c8244612 100644
--- a/tests/managers_regress/tests.py
+++ b/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):
diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py
index 59cd56e78b..13514cac91 100644
--- a/tests/migrations/test_commands.py
+++ b/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)
diff --git a/tests/proxy_model_inheritance/tests.py b/tests/proxy_model_inheritance/tests.py
index 861ab4af17..601fdc5b42 100644
--- a/tests/proxy_model_inheritance/tests.py
+++ b/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:
diff --git a/tests/proxy_models/tests.py b/tests/proxy_models/tests.py
index 3389f3597f..0995a778c0 100644
--- a/tests/proxy_models/tests.py
+++ b/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):
diff --git a/tests/tablespaces/tests.py b/tests/tablespaces/tests.py
index 4a62ad5a45..fa90704c45 100644
--- a/tests/tablespaces/tests.py
+++ b/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):