Skip to content

Commit

Permalink
Terminated AppCache._populate() with extreme prejudice.
Browse files Browse the repository at this point in the history
It was called _populate() before I renamed it to populate(). Since it
has been superseded by populate_models() there's no reason to keep it.

Removed the can_postpone argument of load_app() as it was only used by
populate(). It's a private API and there's no replacement. Simplified
load_app() accordingly. Then new version behaves exactly like the old
one even though it's much shorter.
  • Loading branch information
aaugustin committed Dec 22, 2013
1 parent 2b56d69 commit 86804ab
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 84 deletions.
2 changes: 1 addition & 1 deletion django/contrib/admin/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class BaseValidator(object):
def __init__(self):
# Before we can introspect models, they need the app cache to be fully
# loaded so that inter-relations are set up correctly.
app_cache.populate()
app_cache.populate_models()

def validate(self, cls, model):
for m in dir(self):
Expand Down
89 changes: 14 additions & 75 deletions django/core/apps/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@

from collections import defaultdict, OrderedDict
from contextlib import contextmanager
from importlib import import_module
import os
import sys
import warnings

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.module_loading import import_lock
from django.utils._os import upath

from .base import AppConfig, MODELS_MODULE_NAME
from .base import AppConfig


class UnavailableApp(Exception):
Expand Down Expand Up @@ -51,15 +50,11 @@ 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
# Internal flags used when populating the master cache.
self._apps_loaded = not self.master
self._models_loaded = not self.master

# -- Everything below here is only used when populating the cache --
self.loaded = False
self.handled = set()
self.postponed = []
self.nesting_level = 0
# Cache for get_models.
self._get_models_cache = {}

def populate_apps(self):
Expand Down Expand Up @@ -138,71 +133,15 @@ def populate_models(self):

self._models_loaded = True

def populate(self):
"""
Fill in all the cache information. This method is threadsafe, in the
sense that every caller will see the same state upon return, and if the
cache is already initialised, it does no work.
"""
if self.loaded:
return
if not self.master:
self.loaded = True
return
# Note that we want to use the import lock here - the app loading is
# in many cases initiated implicitly by importing, and thus it is
# possible to end up in deadlock when one thread initiates loading
# without holding the importer lock and another thread then tries to
# import something which also launches the app loading. For details of
# this situation see #18251.
with import_lock():
if self.loaded:
return
for app_name in settings.INSTALLED_APPS:
if app_name in self.handled:
continue
self.load_app(app_name, can_postpone=True)
if not self.nesting_level:
for app_name in self.postponed:
self.load_app(app_name)
self.loaded = True

def load_app(self, app_name, can_postpone=False):
def load_app(self, app_name):
"""
Loads the app with the provided fully qualified name, and returns the
model module.
"""
app_module = import_module(app_name)
self.handled.add(app_name)
self.nesting_level += 1
try:
models_module = import_module('%s.%s' % (app_name, MODELS_MODULE_NAME))
except ImportError:
# If the app doesn't have a models module, we can just swallow the
# ImportError and return no models for this app.
if not module_has_submodule(app_module, MODELS_MODULE_NAME):
models_module = None
# But if the app does have a models module, we need to figure out
# whether to suppress or propagate the error. If can_postpone is
# True then it may be that the package is still being imported by
# Python and the models module isn't available yet. So we add the
# app to the postponed list and we'll try it again after all the
# recursion has finished (in populate). If can_postpone is False
# then it's time to raise the ImportError.
else:
if can_postpone:
self.postponed.append(app_name)
return
else:
raise
finally:
self.nesting_level -= 1

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
return app_config.models_module

def app_cache_ready(self):
"""
Expand All @@ -211,7 +150,7 @@ def app_cache_ready(self):
Useful for code that wants to cache the results of get_models() for
themselves once it is safe to do so.
"""
return self.loaded
return self._models_loaded # implies self._apps_loaded.

def get_app_configs(self, only_with_models_module=False):
"""
Expand All @@ -220,7 +159,7 @@ def get_app_configs(self, only_with_models_module=False):
If only_with_models_module in True (non-default), only applications
containing a models module are considered.
"""
self.populate()
self.populate_models()
for app_config in self.app_configs.values():
if only_with_models_module and app_config.models_module is None:
continue
Expand All @@ -240,7 +179,7 @@ def get_app_config(self, app_label, only_with_models_module=False):
If only_with_models_module in True (non-default), only applications
containing a models module are considered.
"""
self.populate()
self.populate_models()
app_config = self.app_configs.get(app_label)
if app_config is None:
raise LookupError("No installed app with label %r." % app_label)
Expand Down Expand Up @@ -288,7 +227,7 @@ def get_models(self, app_mod=None,
return model_list
except KeyError:
pass
self.populate()
self.populate_models()
if app_mod:
app_label = app_mod.__name__.split('.')[-2]
if only_installed:
Expand Down Expand Up @@ -331,7 +270,7 @@ def get_model(self, app_label, model_name, only_installed=True):
"""
if not self.master:
only_installed = False
self.populate()
self.populate_models()
if only_installed:
app_config = self.app_configs.get(app_label)
if app_config is None:
Expand Down Expand Up @@ -496,7 +435,7 @@ def get_app_paths(self):
"[a.path for a in get_app_configs()] supersedes get_app_paths().",
PendingDeprecationWarning, stacklevel=2)

self.populate()
self.populate_models()

app_paths = []
for app in self.get_apps():
Expand Down
2 changes: 1 addition & 1 deletion django/core/serializers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def __init__(self, stream_or_string, **options):
self.stream = stream_or_string
# Make sure the app cache is loaded before deserialization starts
# (otherwise subclass calls to get_model() and friends might fail...)
app_cache.populate()
app_cache.populate_models()

def __iter__(self):
return self
Expand Down
2 changes: 1 addition & 1 deletion django/core/serializers/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def Deserializer(object_list, **options):
db = options.pop('using', DEFAULT_DB_ALIAS)
ignore = options.pop('ignorenonexistent', False)

app_cache.populate()
app_cache.populate_models()

for d in object_list:
# Look up the model and starting build a dict of data for it.
Expand Down
2 changes: 1 addition & 1 deletion django/db/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.utils.six.moves import zip

from django.core.apps import app_cache
from django.core.apps.cache import MODELS_MODULE_NAME
from django.core.apps.base import MODELS_MODULE_NAME
import django.db.models.manager # NOQA: Imported to register signal handler.
from django.conf import settings
from django.core.exceptions import (ObjectDoesNotExist,
Expand Down
2 changes: 1 addition & 1 deletion django/db/models/loading.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ def get_app_errors():
try:
return app_cache.app_errors
except AttributeError:
app_cache.populate()
app_cache.populate_models()
app_cache.app_errors = {}
return app_cache.app_errors
2 changes: 1 addition & 1 deletion django/db/models/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def __init__(self, meta, app_label=None):

@property
def app_config(self):
# Don't go through get_app_config to avoid triggering populate().
# Don't go through get_app_config to avoid triggering imports.
return self.app_cache.app_configs.get(self.app_label)

@property
Expand Down
5 changes: 3 additions & 2 deletions tests/app_loading/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ def test_missing_app(self):
error. Refs #17667.
"""
app_cache = AppCache()
# Pretend we're the master app cache to test populate().
app_cache.master = True
# Pretend we're the master app cache to test the population process.
app_cache._apps_loaded = False
app_cache._models_loaded = False
with override_settings(INSTALLED_APPS=('notexists',)):
with self.assertRaises(ImportError):
app_cache.get_model('notexists', 'nomodel')
Expand Down
2 changes: 1 addition & 1 deletion tests/runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def no_available_apps(self):
# Load all the ALWAYS_INSTALLED_APPS.
with warnings.catch_warnings():
warnings.filterwarnings('ignore', 'django.contrib.comments is deprecated and will be removed before Django 1.8.', DeprecationWarning)
app_cache.populate()
app_cache.populate_models()

# Load all the test model apps.
test_modules = get_test_modules()
Expand Down

0 comments on commit 86804ab

Please sign in to comment.