From 5891990b6e6f6e90a873ceb199b321177a90c9eb Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 23 Dec 2013 00:10:53 +0100 Subject: [PATCH] Refactored INSTALLED_APPS overrides. * 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. --- django/apps/__init__.py | 4 +- django/apps/cache.py | 88 +++++++++++----------- django/contrib/auth/management/__init__.py | 13 ++-- django/contrib/contenttypes/management.py | 6 +- django/test/signals.py | 10 ++- django/test/testcases.py | 18 ++++- django/test/utils.py | 7 ++ tests/app_loading/tests.py | 20 ----- 8 files changed, 85 insertions(+), 81 deletions(-) diff --git a/django/apps/__init__.py b/django/apps/__init__.py index a1193eafcc..6b47d3397f 100644 --- a/django/apps/__init__.py +++ b/django/apps/__init__.py @@ -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 diff --git a/django/apps/cache.py b/django/apps/cache.py index 2c27f96a72..bdd7e98e22 100644 --- a/django/apps/cache.py +++ b/django/apps/cache.py @@ -14,10 +14,6 @@ from django.utils._os import upath from .base import AppConfig -class UnavailableApp(Exception): - pass - - class AppCache(object): """ A cache that stores installed applications and their models. Used to @@ -43,9 +39,9 @@ class AppCache(object): # 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 @@ -157,8 +153,6 @@ class AppCache(object): 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): @@ -167,9 +161,6 @@ class AppCache(object): 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. """ @@ -183,8 +174,6 @@ class AppCache(object): 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, @@ -216,13 +205,7 @@ class AppCache(object): 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() @@ -249,11 +232,6 @@ class AppCache(object): (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): @@ -262,9 +240,6 @@ class AppCache(object): 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 @@ -273,9 +248,6 @@ class AppCache(object): 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): @@ -326,22 +298,57 @@ class AppCache(object): 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) @@ -353,15 +360,11 @@ class AppCache(object): 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): @@ -420,9 +423,6 @@ class AppCache(object): 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).", diff --git a/django/contrib/auth/management/__init__.py b/django/contrib/auth/management/__init__.py index 5f24bf069e..008661fac6 100644 --- a/django/contrib/auth/management/__init__.py +++ b/django/contrib/auth/management/__init__.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals 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 @@ -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): @@ -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): diff --git a/django/contrib/contenttypes/management.py b/django/contrib/contenttypes/management.py index 35a8a7fdc8..4ea10ebe58 100644 --- a/django/contrib/contenttypes/management.py +++ b/django/contrib/contenttypes/management.py @@ -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 @@ -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): diff --git a/django/test/signals.py b/django/test/signals.py index 0d6b7bedf1..03c40f259d 100644 --- a/django/test/signals.py +++ b/django/test/signals.py @@ -17,7 +17,7 @@ setting_changed = Signal(providing_args=["setting", "value", "enter"]) # 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) @@ -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': diff --git a/django/test/testcases.py b/django/test/testcases.py index 1480889565..1532ec433f 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -30,7 +30,7 @@ from django.forms.fields import CharField 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 @@ -726,6 +726,10 @@ class TransactionTestCase(SimpleTestCase): 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: @@ -733,6 +737,11 @@ class TransactionTestCase(SimpleTestCase): 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): @@ -786,7 +795,12 @@ class TransactionTestCase(SimpleTestCase): 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 diff --git a/django/test/utils.py b/django/test/utils.py index 550d7642b9..ce7536bd6d 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -9,6 +9,7 @@ import warnings 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 @@ -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() @@ -223,6 +226,8 @@ class override_settings(object): 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) @@ -230,6 +235,8 @@ class override_settings(object): 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__, diff --git a/tests/app_loading/tests.py b/tests/app_loading/tests.py index 0692597a5b..164e363161 100644 --- a/tests/app_loading/tests.py +++ b/tests/app_loading/tests.py @@ -3,11 +3,8 @@ from __future__ import unicode_literals 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 @@ -69,23 +66,6 @@ class EggLoadingTest(TestCase): 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):