From 9cdf1f6d547b6dffcee26f0a525429452551bb4a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 19 Dec 2013 21:57:09 +0100 Subject: [PATCH] Stop testing for inclusion in INSTALLED_APPS. Removed some exception masking in the comments app that was harmful and couldn't be preserved easily. --- .../admin/templatetags/admin_static.py | 4 +- django/contrib/comments/__init__.py | 19 +++---- django/contrib/messages/tests/base.py | 53 +++++++++---------- django/contrib/redirects/middleware.py | 3 +- django/contrib/redirects/tests.py | 9 ++-- .../contrib/sitemaps/tests/test_flatpages.py | 3 +- django/contrib/sitemaps/tests/test_http.py | 2 +- django/core/apps/cache.py | 12 +++++ django/test/client.py | 5 +- tests/comment_tests/tests/test_app_api.py | 21 +++----- 10 files changed, 66 insertions(+), 65 deletions(-) diff --git a/django/contrib/admin/templatetags/admin_static.py b/django/contrib/admin/templatetags/admin_static.py index 5ea3ba5838..75b077057e 100644 --- a/django/contrib/admin/templatetags/admin_static.py +++ b/django/contrib/admin/templatetags/admin_static.py @@ -1,9 +1,9 @@ -from django.conf import settings +from django.core.apps import app_cache from django.template import Library register = Library() -if 'django.contrib.staticfiles' in settings.INSTALLED_APPS: +if app_cache.has_app('django.contrib.staticfiles'): from django.contrib.staticfiles.templatetags.staticfiles import static else: from django.templatetags.static import static diff --git a/django/contrib/comments/__init__.py b/django/contrib/comments/__init__.py index 56ed32bafe..8d4b922966 100644 --- a/django/contrib/comments/__init__.py +++ b/django/contrib/comments/__init__.py @@ -2,6 +2,7 @@ from importlib import import_module import warnings from django.conf import settings from django.core import urlresolvers +from django.core.apps import app_cache from django.core.exceptions import ImproperlyConfigured from django.contrib.comments.models import Comment from django.contrib.comments.forms import CommentForm @@ -14,20 +15,12 @@ def get_comment_app(): """ Get the comment app (i.e. "django.contrib.comments") as defined in the settings """ - # Make sure the app's in INSTALLED_APPS - comments_app = get_comment_app_name() - if comments_app not in settings.INSTALLED_APPS: - raise ImproperlyConfigured("The COMMENTS_APP (%r) "\ - "must be in INSTALLED_APPS" % settings.COMMENTS_APP) - - # Try to import the package try: - package = import_module(comments_app) - except ImportError as e: - raise ImproperlyConfigured("The COMMENTS_APP setting refers to "\ - "a non-existing package. (%s)" % e) - - return package + app_config = app_cache.get_app_config(get_comment_app_name().rpartition(".")[2]) + except LookupError: + raise ImproperlyConfigured("The COMMENTS_APP (%r) " + "must be in INSTALLED_APPS" % settings.COMMENTS_APP) + return app_config.app_module def get_comment_app_name(): """ diff --git a/django/contrib/messages/tests/base.py b/django/contrib/messages/tests/base.py index f3e676fac5..8d7906ba52 100644 --- a/django/contrib/messages/tests/base.py +++ b/django/contrib/messages/tests/base.py @@ -1,4 +1,4 @@ -from unittest import skipIf +from unittest import skipUnless from django import http from django.conf import settings, global_settings @@ -7,14 +7,15 @@ from django.contrib.messages.api import MessageFailure from django.contrib.messages.constants import DEFAULT_LEVELS from django.contrib.messages.storage import default_storage, base from django.contrib.messages.storage.base import Message +from django.core.apps import app_cache from django.core.urlresolvers import reverse from django.test.utils import override_settings from django.utils.translation import ugettext_lazy def skipUnlessAuthIsInstalled(func): - return skipIf( - 'django.contrib.auth' not in settings.INSTALLED_APPS, + return skipUnless( + app_cache.has_app('django.contrib.auth'), "django.contrib.auth isn't installed")(func) @@ -219,8 +220,6 @@ class BaseTests(object): self.assertContains(response, msg) @override_settings( - INSTALLED_APPS=filter( - lambda app: app != 'django.contrib.messages', settings.INSTALLED_APPS), MIDDLEWARE_CLASSES=filter( lambda m: 'MessageMiddleware' not in m, settings.MIDDLEWARE_CLASSES), TEMPLATE_CONTEXT_PROCESSORS=filter( @@ -233,19 +232,18 @@ class BaseTests(object): Tests that, when the middleware is disabled, an exception is raised when one attempts to store a message. """ - data = { - 'messages': ['Test message %d' % x for x in range(5)], - } - reverse('django.contrib.messages.tests.urls.show') - for level in ('debug', 'info', 'success', 'warning', 'error'): - add_url = reverse('django.contrib.messages.tests.urls.add', - args=(level,)) - self.assertRaises(MessageFailure, self.client.post, add_url, - data, follow=True) + with app_cache._without_app('django.contrib.messages'): + data = { + 'messages': ['Test message %d' % x for x in range(5)], + } + reverse('django.contrib.messages.tests.urls.show') + for level in ('debug', 'info', 'success', 'warning', 'error'): + add_url = reverse('django.contrib.messages.tests.urls.add', + args=(level,)) + self.assertRaises(MessageFailure, self.client.post, add_url, + data, follow=True) @override_settings( - INSTALLED_APPS=filter( - lambda app: app != 'django.contrib.messages', settings.INSTALLED_APPS), MIDDLEWARE_CLASSES=filter( lambda m: 'MessageMiddleware' not in m, settings.MIDDLEWARE_CLASSES), TEMPLATE_CONTEXT_PROCESSORS=filter( @@ -258,17 +256,18 @@ class BaseTests(object): Tests that, when the middleware is disabled, an exception is not raised if 'fail_silently' = True """ - data = { - 'messages': ['Test message %d' % x for x in range(5)], - 'fail_silently': True, - } - show_url = reverse('django.contrib.messages.tests.urls.show') - for level in ('debug', 'info', 'success', 'warning', 'error'): - add_url = reverse('django.contrib.messages.tests.urls.add', - args=(level,)) - response = self.client.post(add_url, data, follow=True) - self.assertRedirects(response, show_url) - self.assertFalse('messages' in response.context) + with app_cache._without_app('django.contrib.messages'): + data = { + 'messages': ['Test message %d' % x for x in range(5)], + 'fail_silently': True, + } + show_url = reverse('django.contrib.messages.tests.urls.show') + for level in ('debug', 'info', 'success', 'warning', 'error'): + add_url = reverse('django.contrib.messages.tests.urls.add', + args=(level,)) + response = self.client.post(add_url, data, follow=True) + self.assertRedirects(response, show_url) + self.assertFalse('messages' in response.context) def stored_messages_count(self, storage, response): """ diff --git a/django/contrib/redirects/middleware.py b/django/contrib/redirects/middleware.py index 9053c64a9a..e8aa06ebb1 100644 --- a/django/contrib/redirects/middleware.py +++ b/django/contrib/redirects/middleware.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals from django.conf import settings from django.contrib.redirects.models import Redirect from django.contrib.sites.models import get_current_site +from django.core.apps import app_cache from django.core.exceptions import ImproperlyConfigured from django import http @@ -14,7 +15,7 @@ class RedirectFallbackMiddleware(object): response_redirect_class = http.HttpResponsePermanentRedirect def __init__(self): - if 'django.contrib.sites' not in settings.INSTALLED_APPS: + if not app_cache.has_app('django.contrib.sites'): raise ImproperlyConfigured( "You cannot use RedirectFallbackMiddleware when " "django.contrib.sites is not installed." diff --git a/django/contrib/redirects/tests.py b/django/contrib/redirects/tests.py index 84638d6ab6..8566a34c68 100644 --- a/django/contrib/redirects/tests.py +++ b/django/contrib/redirects/tests.py @@ -1,6 +1,7 @@ from django import http from django.conf import settings from django.contrib.sites.models import Site +from django.core.apps import app_cache from django.core.exceptions import ImproperlyConfigured from django.test import TestCase from django.test.utils import override_settings @@ -56,12 +57,10 @@ class RedirectTests(TestCase): response = self.client.get('/initial') self.assertEqual(response.status_code, 410) - @override_settings( - INSTALLED_APPS=[app for app in settings.INSTALLED_APPS - if app != 'django.contrib.sites']) def test_sites_not_installed(self): - with self.assertRaises(ImproperlyConfigured): - RedirectFallbackMiddleware() + with app_cache._without_app('django.contrib.sites'): + with self.assertRaises(ImproperlyConfigured): + RedirectFallbackMiddleware() class OverriddenRedirectFallbackMiddleware(RedirectFallbackMiddleware): diff --git a/django/contrib/sitemaps/tests/test_flatpages.py b/django/contrib/sitemaps/tests/test_flatpages.py index 75ebb10f96..33ba168a45 100644 --- a/django/contrib/sitemaps/tests/test_flatpages.py +++ b/django/contrib/sitemaps/tests/test_flatpages.py @@ -3,13 +3,14 @@ from __future__ import unicode_literals from unittest import skipUnless from django.conf import settings +from django.core.apps import app_cache from .base import SitemapTestsBase class FlatpagesSitemapTests(SitemapTestsBase): - @skipUnless("django.contrib.flatpages" in settings.INSTALLED_APPS, + @skipUnless(app_cache.has_app('django.contrib.flatpages'), "django.contrib.flatpages app not installed.") def test_flatpage_sitemap(self): "Basic FlatPage sitemap test" diff --git a/django/contrib/sitemaps/tests/test_http.py b/django/contrib/sitemaps/tests/test_http.py index ed61c9950b..94d9adea49 100644 --- a/django/contrib/sitemaps/tests/test_http.py +++ b/django/contrib/sitemaps/tests/test_http.py @@ -118,7 +118,7 @@ class HTTPSitemapTests(SitemapTestsBase): """ % date.today() self.assertXMLEqual(response.content.decode('utf-8'), expected_content) - @skipUnless("django.contrib.sites" in settings.INSTALLED_APPS, + @skipUnless(app_cache.has_app('django.contrib.sites'), "django.contrib.sites app not installed.") def test_sitemap_get_urls_no_site_1(self): """ diff --git a/django/core/apps/cache.py b/django/core/apps/cache.py index 41196e5f05..697cf9e516 100644 --- a/django/core/apps/cache.py +++ b/django/core/apps/cache.py @@ -298,6 +298,18 @@ class AppCache(object): models[model_name] = model self._get_models_cache.clear() + def has_app(self, app_name): + """ + Returns the application config if one is registered and None otherwise. + + It's safe to call this method at import time, even while the app cache + is being populated. It returns None for apps that aren't loaded yet. + """ + app_config = self.app_configs.get(app_name.rpartition(".")[2]) + if app_config is not None and app_config.name != app_name: + app_config = None + return app_config + def has_model(self, app_label, model_name): """ Returns the model class if one is registered and None otherwise. diff --git a/django/test/client.py b/django/test/client.py index 8c9a17564c..cccc3ecc42 100644 --- a/django/test/client.py +++ b/django/test/client.py @@ -10,6 +10,7 @@ from io import BytesIO from django.conf import settings from django.contrib.auth import authenticate, login, logout, get_user_model +from django.core.apps import app_cache from django.core.handlers.base import BaseHandler from django.core.handlers.wsgi import WSGIRequest from django.core.signals import (request_started, request_finished, @@ -389,7 +390,7 @@ class Client(RequestFactory): """ Obtains the current session variables. """ - if 'django.contrib.sessions' in settings.INSTALLED_APPS: + if app_cache.has_app('django.contrib.sessions'): engine = import_module(settings.SESSION_ENGINE) cookie = self.cookies.get(settings.SESSION_COOKIE_NAME, None) if cookie: @@ -550,7 +551,7 @@ class Client(RequestFactory): """ user = authenticate(**credentials) if (user and user.is_active and - 'django.contrib.sessions' in settings.INSTALLED_APPS): + app_cache.has_app('django.contrib.sessions')): engine = import_module(settings.SESSION_ENGINE) # Create a fake request that goes through request middleware diff --git a/tests/comment_tests/tests/test_app_api.py b/tests/comment_tests/tests/test_app_api.py index 384802d1b2..645daf123d 100644 --- a/tests/comment_tests/tests/test_app_api.py +++ b/tests/comment_tests/tests/test_app_api.py @@ -2,6 +2,7 @@ from django.conf import settings from django.contrib import comments from django.contrib.comments.models import Comment from django.contrib.comments.forms import CommentForm +from django.core.apps import app_cache from django.core.exceptions import ImproperlyConfigured from django.test.utils import override_settings from django.utils import six @@ -15,14 +16,6 @@ class CommentAppAPITests(CommentTestCase): def testGetCommentApp(self): self.assertEqual(comments.get_comment_app(), comments) - @override_settings( - COMMENTS_APP='missing_app', - INSTALLED_APPS=list(settings.INSTALLED_APPS) + ['missing_app'], - ) - def testGetMissingCommentApp(self): - with six.assertRaisesRegex(self, ImproperlyConfigured, 'missing_app'): - comments.get_comment_app() - def testGetForm(self): self.assertEqual(comments.get_form(), CommentForm) @@ -42,14 +35,16 @@ class CommentAppAPITests(CommentTestCase): self.assertEqual(comments.get_approve_url(c), "/approve/12345/") -@override_settings( - COMMENTS_APP='comment_tests.custom_comments', - INSTALLED_APPS=list(settings.INSTALLED_APPS) + [ - 'comment_tests.custom_comments'], -) +@override_settings(COMMENTS_APP='comment_tests.custom_comments') class CustomCommentTest(CommentTestCase): urls = 'comment_tests.urls' + def setUp(self): + self._with_custom_comments = app_cache._begin_with_app('comment_tests.custom_comments') + + def tearDown(self): + app_cache._end_with_app(self._with_custom_comments) + def testGetCommentApp(self): from comment_tests import custom_comments self.assertEqual(comments.get_comment_app(), custom_comments)