Browse Source

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.
Aymeric Augustin 11 năm trước cách đây
mục cha
commit
5891990b6e

+ 2 - 2
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

+ 44 - 44
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).",

+ 5 - 8
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):

+ 2 - 4
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):

+ 9 - 1
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':

+ 16 - 2
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

+ 7 - 0
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__,

+ 0 - 20
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):