Skip to content

Commit

Permalink
Refactored INSTALLED_APPS overrides.
Browse files Browse the repository at this point in the history
* Introduced [un]set_installed_apps to handle changes to the
  INSTALLED_APPS setting.
* Refactored [un]set_available_apps to share its implementation
  with [un]set_installed_apps.
* Implemented a receiver to clear some app-related caches.
* Removed test_missing_app as it is basically impossible to reproduce
  this situation with public methods of the new app cache.
  • Loading branch information
aaugustin committed Dec 23, 2013
1 parent 8cff95e commit 5891990
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 81 deletions.
4 changes: 2 additions & 2 deletions django/apps/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
from .base import AppConfig # NOQA
from .cache import app_cache, UnavailableApp # NOQA
from .base import AppConfig # NOQA
from .cache import app_cache # NOQA
88 changes: 44 additions & 44 deletions django/apps/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@
from .base import AppConfig


class UnavailableApp(Exception):
pass


class AppCache(object):
"""
A cache that stores installed applications and their models. Used to
Expand All @@ -43,9 +39,9 @@ def __init__(self, master=False):
# Mapping of labels to AppConfig instances for installed apps.
self.app_configs = OrderedDict()

# Set of app names. Allows restricting the set of installed apps.
# Used by TransactionTestCase.available_apps for performance reasons.
self.available_apps = None
# Stack of app_configs. Used to store the current state in
# set_available_apps and set_installed_apps.
self.stored_app_configs = []

# Internal flags used when populating the master cache.
self._apps_loaded = not self.master
Expand Down Expand Up @@ -157,8 +153,6 @@ def get_app_configs(self, only_with_models_module=False):
for app_config in self.app_configs.values():
if only_with_models_module and app_config.models_module is None:
continue
if self.available_apps is not None and app_config.name not in self.available_apps:
continue
yield app_config

def get_app_config(self, app_label, only_with_models_module=False):
Expand All @@ -167,9 +161,6 @@ def get_app_config(self, app_label, only_with_models_module=False):
Raises LookupError if no application exists with this label.
Raises UnavailableApp when set_available_apps() disables the
application with this label.
If only_with_models_module in True (non-default), imports models and
considers only applications containing a models module.
"""
Expand All @@ -183,8 +174,6 @@ def get_app_config(self, app_label, only_with_models_module=False):
raise LookupError("No installed app with label %r." % app_label)
if only_with_models_module and app_config.models_module is None:
raise LookupError("App with label %r doesn't have a models module." % app_label)
if self.available_apps is not None and app_config.name not in self.available_apps:
raise UnavailableApp("App with label %r isn't available." % app_label)
return app_config

def get_models(self, app_mod=None,
Expand Down Expand Up @@ -216,13 +205,7 @@ def get_models(self, app_mod=None,
cache_key = (app_mod, include_auto_created, include_deferred, only_installed, include_swapped)
model_list = None
try:
model_list = self._get_models_cache[cache_key]
if self.available_apps is not None and only_installed:
model_list = [
m for m in model_list
if self.app_configs[m._meta.app_label].name in self.available_apps
]
return model_list
return self._get_models_cache[cache_key]
except KeyError:
pass
self.populate_models()
Expand All @@ -249,11 +232,6 @@ def get_models(self, app_mod=None,
(not model._meta.swapped or include_swapped))
)
self._get_models_cache[cache_key] = model_list
if self.available_apps is not None and only_installed:
model_list = [
m for m in model_list
if self.app_configs[m._meta.app_label].name in self.available_apps
]
return model_list

def get_model(self, app_label, model_name, only_installed=True):
Expand All @@ -262,9 +240,6 @@ def get_model(self, app_label, model_name, only_installed=True):
model_name.
Returns None if no model is found.
Raises UnavailableApp when set_available_apps() in in effect and
doesn't include app_label.
"""
if not self.master:
only_installed = False
Expand All @@ -273,9 +248,6 @@ def get_model(self, app_label, model_name, only_installed=True):
app_config = self.app_configs.get(app_label)
if app_config is None:
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)
return self.all_models[app_label].get(model_name.lower())

def register_model(self, app_label, model):
Expand Down Expand Up @@ -326,22 +298,57 @@ def set_available_apps(self, available):
available must be an iterable of application names.
Primarily used for performance optimization in TransactionTestCase.
This method is safe is the sense that it doesn't trigger any imports.
"""
if self.available_apps is not None:
raise RuntimeError("set_available_apps() may be called only once "
"in a row; make sure it's paired with unset_available_apps()")
available = set(available)
installed = set(app_config.name for app_config in self.get_app_configs())
if not available.issubset(installed):
raise ValueError("Available apps isn't a subset of installed "
"apps, extra apps: %s" % ", ".join(available - installed))
self.available_apps = available

self.stored_app_configs.append(self.app_configs)
self.app_configs = OrderedDict(
(label, app_config)
for label, app_config in self.app_configs.items()
if app_config.name in available)

def unset_available_apps(self):
"""
Cancels a previous call to set_available_apps().
"""
self.available_apps = None
self.app_configs = self.stored_app_configs.pop()

def set_installed_apps(self, installed):
"""
Enables a different set of installed_apps for get_app_config[s].
installed must be an iterable in the same format as INSTALLED_APPS.
Primarily used as a receiver of the setting_changed signal in tests.
This method may trigger new imports, which may add new models to the
registry of all imported models. They will stay in the registry even
after unset_installed_apps(). Since it isn't possible to replay
imports safely (eg. that could lead to registering listeners twice),
models are registered when they're imported and never removed.
"""
self.stored_app_configs.append(self.app_configs)
self.app_configs = OrderedDict()
try:
self._apps_loaded = False
self.populate_apps()
self._models_loaded = False
self.populate_models()
except Exception:
self.unset_installed_apps()
raise

def unset_installed_apps(self):
"""
Cancels a previous call to set_installed_apps().
"""
self.app_configs = self.stored_app_configs.pop()

### DANGEROUS METHODS ### (only used to preserve existing tests)

Expand All @@ -353,15 +360,11 @@ def _begin_with_app(self, app_name):
else:
app_config.import_models(self.all_models[app_config.label])
self.app_configs[app_config.label] = app_config
if self.available_apps is not None:
self.available_apps.add(app_config.name)
return app_config

def _end_with_app(self, app_config):
if app_config is not None:
del self.app_configs[app_config.label]
if self.available_apps is not None:
self.available_apps.discard(app_config.name)

@contextmanager
def _with_app(self, app_name):
Expand Down Expand Up @@ -420,9 +423,6 @@ def load_app(self, app_name):
def get_app(self, app_label):
"""
Returns the module containing the models for the given app_label.
Raises UnavailableApp when set_available_apps() in in effect and
doesn't include app_label.
"""
warnings.warn(
"get_app_config(app_label).models_module supersedes get_app(app_label).",
Expand Down
13 changes: 5 additions & 8 deletions django/contrib/auth/management/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import getpass
import unicodedata

from django.apps import app_cache, UnavailableApp
from django.apps import app_cache
from django.contrib.auth import (models as auth_app, get_permission_codename,
get_user_model)
from django.core import exceptions
Expand Down Expand Up @@ -61,9 +61,7 @@ def _check_permission_clashing(custom, builtin, ctype):


def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kwargs):
try:
app_cache.get_model('auth', 'Permission')
except UnavailableApp:
if app_cache.get_model('auth', 'Permission') is None:
return

if not router.allow_migrate(db, auth_app.Permission):
Expand Down Expand Up @@ -119,12 +117,11 @@ def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kw


def create_superuser(app, created_models, verbosity, db, **kwargs):
try:
app_cache.get_model('auth', 'Permission')
UserModel = get_user_model()
except UnavailableApp:
if app_cache.get_model('auth', 'Permission') is None:
return

UserModel = get_user_model()

from django.core.management import call_command

if UserModel in created_models and kwargs.get('interactive', True):
Expand Down
6 changes: 2 additions & 4 deletions django/contrib/contenttypes/management.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from django.apps import app_cache, UnavailableApp
from django.apps import app_cache
from django.contrib.contenttypes.models import ContentType
from django.db import DEFAULT_DB_ALIAS, router
from django.db.models import signals
Expand All @@ -12,9 +12,7 @@ def update_contenttypes(app, created_models, verbosity=2, db=DEFAULT_DB_ALIAS, *
Creates content types for models in the given app, removing any model
entries that no longer have a matching model class.
"""
try:
app_cache.get_model('contenttypes', 'ContentType')
except UnavailableApp:
if app_cache.get_model('contenttypes', 'ContentType') is None:
return

if not router.allow_migrate(db, ContentType):
Expand Down
10 changes: 9 additions & 1 deletion django/test/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# except for cases where the receiver is related to a contrib app.

# Settings that may not work well when using 'override_settings' (#19031)
COMPLEX_OVERRIDE_SETTINGS = set(['DATABASES', 'INSTALLED_APPS'])
COMPLEX_OVERRIDE_SETTINGS = set(['DATABASES'])


@receiver(setting_changed)
Expand All @@ -27,6 +27,14 @@ def clear_cache_handlers(**kwargs):
caches._caches = threading.local()


@receiver(setting_changed)
def update_installed_apps(**kwargs):
if kwargs['setting'] == 'INSTALLED_APPS':
# Rebuild any AppDirectoriesFinder instance.
from django.contrib.staticfiles.finders import get_finder
get_finder.cache_clear()


@receiver(setting_changed)
def update_connections_time_zone(**kwargs):
if kwargs['setting'] == 'TIME_ZONE':
Expand Down
18 changes: 16 additions & 2 deletions django/test/testcases.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from django.http import QueryDict
from django.test.client import Client
from django.test.html import HTMLParseError, parse_html
from django.test.signals import template_rendered
from django.test.signals import setting_changed, template_rendered
from django.test.utils import (CaptureQueriesContext, ContextList,
override_settings, compare_xml)
from django.utils.encoding import force_text
Expand Down Expand Up @@ -726,13 +726,22 @@ def _pre_setup(self):
super(TransactionTestCase, self)._pre_setup()
if self.available_apps is not None:
app_cache.set_available_apps(self.available_apps)
setting_changed.send(sender=settings._wrapped.__class__,
setting='INSTALLED_APPS',
value=self.available_apps,
enter=True)
for db_name in self._databases_names(include_mirrors=False):
flush.Command.emit_post_migrate(verbosity=0, interactive=False, database=db_name)
try:
self._fixture_setup()
except Exception:
if self.available_apps is not None:
app_cache.unset_available_apps()
setting_changed.send(sender=settings._wrapped.__class__,
setting='INSTALLED_APPS',
value=settings.INSTALLED_APPS,
enter=False)

raise

def _databases_names(self, include_mirrors=True):
Expand Down Expand Up @@ -786,7 +795,12 @@ def _post_teardown(self):
for conn in connections.all():
conn.close()
finally:
app_cache.unset_available_apps()
if self.available_apps is not None:
app_cache.unset_available_apps()
setting_changed.send(sender=settings._wrapped.__class__,
setting='INSTALLED_APPS',
value=settings.INSTALLED_APPS,
enter=False)

def _fixture_teardown(self):
# Allow TRUNCATE ... CASCADE and don't emit the post_migrate signal
Expand Down
7 changes: 7 additions & 0 deletions django/test/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from functools import wraps
from xml.dom.minidom import parseString, Node

from django.apps import app_cache
from django.conf import settings, UserSettingsHolder
from django.core import mail
from django.core.signals import request_started
Expand Down Expand Up @@ -190,6 +191,8 @@ class override_settings(object):
"""
def __init__(self, **kwargs):
self.options = kwargs
# Special case that requires updating the app cache, a core feature.
self.installed_apps = self.options.get('INSTALLED_APPS')

def __enter__(self):
self.enable()
Expand Down Expand Up @@ -223,13 +226,17 @@ def enable(self):
setattr(override, key, new_value)
self.wrapped = settings._wrapped
settings._wrapped = override
if self.installed_apps is not None:
app_cache.set_installed_apps(self.installed_apps)
for key, new_value in self.options.items():
setting_changed.send(sender=settings._wrapped.__class__,
setting=key, value=new_value, enter=True)

def disable(self):
settings._wrapped = self.wrapped
del self.wrapped
if self.installed_apps is not None:
app_cache.unset_installed_apps()
for key in self.options:
new_value = getattr(settings, key, None)
setting_changed.send(sender=settings._wrapped.__class__,
Expand Down
20 changes: 0 additions & 20 deletions tests/app_loading/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@
import os
import sys
from unittest import TestCase
import warnings

from django.apps import app_cache
from django.apps.cache import AppCache
from django.test.utils import override_settings
from django.utils._os import upath
from django.utils import six

Expand Down Expand Up @@ -69,23 +66,6 @@ def test_egg5(self):
with app_cache._with_app('broken_app'):
app_cache.get_app_config('omelet.app_no_models').models_module

def test_missing_app(self):
"""
Test that repeated app loading doesn't succeed in case there is an
error. Refs #17667.
"""
app_cache = AppCache()
# Pretend we're the master app cache to test the population process.
app_cache._apps_loaded = False
app_cache._models_loaded = False
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "Overriding setting INSTALLED_APPS")
with override_settings(INSTALLED_APPS=['notexists']):
with self.assertRaises(ImportError):
app_cache.get_model('notexists', 'nomodel')
with self.assertRaises(ImportError):
app_cache.get_model('notexists', 'nomodel')


class GetModelsTest(TestCase):
def setUp(self):
Expand Down

0 comments on commit 5891990

Please sign in to comment.