From 9b3389b7268e48c16ed79256e516af787ad652df Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Wed, 18 Dec 2013 13:16:33 +0100 Subject: [PATCH] Removed the app_config.installed flag. Since applications that aren't installed no longer have an application configuration, it is now always True in practice. Provided an abstraction to temporarily add or remove applications as several tests messed with app_config.installed to achieve this effect. For now this API is _-prefixed because it looks dangerous. --- django/contrib/contenttypes/tests.py | 23 ++++--- django/contrib/gis/tests/geoapp/test_feeds.py | 6 +- .../contrib/gis/tests/geoapp/test_sitemaps.py | 6 +- django/contrib/sitemaps/tests/base.py | 4 -- django/contrib/sitemaps/tests/test_http.py | 15 +++-- django/contrib/sites/tests.py | 14 ++--- django/core/apps/base.py | 6 +- django/core/apps/cache.py | 63 ++++++++++++++++++- django/db/models/options.py | 4 +- tests/admin_docs/tests.py | 18 ++---- 10 files changed, 99 insertions(+), 60 deletions(-) diff --git a/django/contrib/contenttypes/tests.py b/django/contrib/contenttypes/tests.py index 756430f393..b194755b80 100644 --- a/django/contrib/contenttypes/tests.py +++ b/django/contrib/contenttypes/tests.py @@ -1,9 +1,10 @@ from __future__ import unicode_literals -from django.db import models from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.views import shortcut -from django.contrib.sites.models import Site, get_current_site +from django.contrib.sites.models import get_current_site +from django.core.apps import app_cache +from django.db import models from django.http import HttpRequest, Http404 from django.test import TestCase from django.test.utils import override_settings @@ -54,11 +55,9 @@ class FooWithBrokenAbsoluteUrl(FooWithoutUrl): class ContentTypesTests(TestCase): def setUp(self): - self._old_installed = Site._meta.app_config.installed ContentType.objects.clear_cache() def tearDown(self): - Site._meta.app_config.installed = self._old_installed ContentType.objects.clear_cache() def test_lookup_cache(self): @@ -223,15 +222,15 @@ class ContentTypesTests(TestCase): user_ct = ContentType.objects.get_for_model(FooWithUrl) obj = FooWithUrl.objects.create(name="john") - Site._meta.app_config.installed = True - response = shortcut(request, user_ct.id, obj.id) - self.assertEqual("http://%s/users/john/" % get_current_site(request).domain, - response._headers.get("location")[1]) + with app_cache._with_app('django.contrib.sites'): + response = shortcut(request, user_ct.id, obj.id) + self.assertEqual("http://%s/users/john/" % get_current_site(request).domain, + response._headers.get("location")[1]) - Site._meta.app_config.installed = False - response = shortcut(request, user_ct.id, obj.id) - self.assertEqual("http://Example.com/users/john/", - response._headers.get("location")[1]) + with app_cache._without_app('django.contrib.sites'): + response = shortcut(request, user_ct.id, obj.id) + self.assertEqual("http://Example.com/users/john/", + response._headers.get("location")[1]) def test_shortcut_view_without_get_absolute_url(self): """ diff --git a/django/contrib/gis/tests/geoapp/test_feeds.py b/django/contrib/gis/tests/geoapp/test_feeds.py index 0817e65cb4..6f0d901a62 100644 --- a/django/contrib/gis/tests/geoapp/test_feeds.py +++ b/django/contrib/gis/tests/geoapp/test_feeds.py @@ -7,6 +7,7 @@ from django.conf import settings from django.contrib.sites.models import Site from django.contrib.gis.geos import HAS_GEOS from django.contrib.gis.tests.utils import HAS_SPATIAL_DB +from django.core.apps import app_cache from django.test import TestCase if HAS_GEOS: @@ -20,11 +21,10 @@ class GeoFeedTest(TestCase): def setUp(self): Site(id=settings.SITE_ID, domain="example.com", name="example.com").save() - self._old_installed = Site._meta.app_config.installed - Site._meta.app_config.installed = True + self._with_sites = app_cache._begin_with_app('django.contrib.sites') def tearDown(self): - Site._meta.app_config.installed = self._old_installed + app_cache._end_with_app(self._with_sites) def assertChildNodes(self, elem, expected): "Taken from syndication/tests.py." diff --git a/django/contrib/gis/tests/geoapp/test_sitemaps.py b/django/contrib/gis/tests/geoapp/test_sitemaps.py index ee9974ceaa..035d97af34 100644 --- a/django/contrib/gis/tests/geoapp/test_sitemaps.py +++ b/django/contrib/gis/tests/geoapp/test_sitemaps.py @@ -10,6 +10,7 @@ from django.conf import settings from django.contrib.gis.geos import HAS_GEOS from django.contrib.gis.tests.utils import HAS_SPATIAL_DB from django.contrib.sites.models import Site +from django.core.apps import app_cache from django.test import TestCase from django.test.utils import IgnoreDeprecationWarningsMixin from django.utils._os import upath @@ -26,11 +27,10 @@ class GeoSitemapTest(IgnoreDeprecationWarningsMixin, TestCase): def setUp(self): super(GeoSitemapTest, self).setUp() Site(id=settings.SITE_ID, domain="example.com", name="example.com").save() - self._old_installed = Site._meta.app_config.installed - Site._meta.app_config.installed = True + self._with_sites = app_cache._begin_with_app('django.contrib.sites') def tearDown(self): - Site._meta.app_config.installed = self._old_installed + app_cache._end_with_app(self._with_sites) super(GeoSitemapTest, self).tearDown() def assertChildNodes(self, elem, expected): diff --git a/django/contrib/sitemaps/tests/base.py b/django/contrib/sitemaps/tests/base.py index 98abb3dca4..ecddcc737b 100644 --- a/django/contrib/sitemaps/tests/base.py +++ b/django/contrib/sitemaps/tests/base.py @@ -25,10 +25,6 @@ class SitemapTestsBase(TestCase): def setUp(self): self.base_url = '%s://%s' % (self.protocol, self.domain) - self._old_installed = Site._meta.app_config.installed cache.clear() # Create an object for sitemap content. TestModel.objects.create(name='Test Object') - - def tearDown(self): - Site._meta.app_config.installed = self._old_installed diff --git a/django/contrib/sitemaps/tests/test_http.py b/django/contrib/sitemaps/tests/test_http.py index 61be8c842a..ed61c9950b 100644 --- a/django/contrib/sitemaps/tests/test_http.py +++ b/django/contrib/sitemaps/tests/test_http.py @@ -7,6 +7,7 @@ from unittest import skipUnless from django.conf import settings from django.contrib.sitemaps import Sitemap, GenericSitemap from django.contrib.sites.models import Site +from django.core.apps import app_cache from django.core.exceptions import ImproperlyConfigured from django.test.utils import override_settings from django.utils.formats import localize @@ -108,15 +109,14 @@ class HTTPSitemapTests(SitemapTestsBase): def test_requestsite_sitemap(self): # Make sure hitting the flatpages sitemap without the sites framework # installed doesn't raise an exception. - # Reset by SitemapTestsBase.tearDown(). - Site._meta.app_config.installed = False - response = self.client.get('/simple/sitemap.xml') - expected_content = """ + with app_cache._without_app('django.contrib.sites'): + response = self.client.get('/simple/sitemap.xml') + expected_content = """ http://testserver/location/%snever0.5 """ % date.today() - self.assertXMLEqual(response.content.decode('utf-8'), expected_content) + self.assertXMLEqual(response.content.decode('utf-8'), expected_content) @skipUnless("django.contrib.sites" in settings.INSTALLED_APPS, "django.contrib.sites app not installed.") @@ -134,9 +134,8 @@ class HTTPSitemapTests(SitemapTestsBase): Sitemap.get_urls if Site objects exists, but the sites framework is not actually installed. """ - # Reset by SitemapTestsBase.tearDown(). - Site._meta.app_config.installed = False - self.assertRaises(ImproperlyConfigured, Sitemap().get_urls) + with app_cache._without_app('django.contrib.sites'): + self.assertRaises(ImproperlyConfigured, Sitemap().get_urls) def test_sitemap_item(self): """ diff --git a/django/contrib/sites/tests.py b/django/contrib/sites/tests.py index 2be28a1429..8d76fd0d02 100644 --- a/django/contrib/sites/tests.py +++ b/django/contrib/sites/tests.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals from django.conf import settings from django.contrib.sites.models import Site, RequestSite, get_current_site +from django.core.apps import app_cache from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.http import HttpRequest from django.test import TestCase @@ -12,11 +13,10 @@ class SitesFrameworkTests(TestCase): def setUp(self): Site(id=settings.SITE_ID, domain="example.com", name="example.com").save() - self._old_installed = Site._meta.app_config.installed - Site._meta.app_config.installed = True + self._with_sites = app_cache._begin_with_app('django.contrib.sites') def tearDown(self): - Site._meta.app_config.installed = self._old_installed + app_cache._end_with_app(self._with_sites) def test_save_another(self): # Regression for #17415 @@ -67,10 +67,10 @@ class SitesFrameworkTests(TestCase): self.assertRaises(ObjectDoesNotExist, get_current_site, request) # A RequestSite is returned if the sites framework is not installed - Site._meta.app_config.installed = False - site = get_current_site(request) - self.assertTrue(isinstance(site, RequestSite)) - self.assertEqual(site.name, "example.com") + with app_cache._without_app('django.contrib.sites'): + site = get_current_site(request) + self.assertTrue(isinstance(site, RequestSite)) + self.assertEqual(site.name, "example.com") def test_domain_name_with_whitespaces(self): # Regression for #17320 diff --git a/django/core/apps/base.py b/django/core/apps/base.py index 5991029c09..332a066bcb 100644 --- a/django/core/apps/base.py +++ b/django/core/apps/base.py @@ -30,14 +30,10 @@ class AppConfig(object): # Populated by calls to AppCache.register_model(). self.models = OrderedDict() - # Whether the app is in INSTALLED_APPS or was automatically created - # when one of its models was imported. - self.installed = app_module is not None - # Filesystem path to the application directory eg. # u'/usr/lib/python2.7/dist-packages/django/contrib/admin'. # This is a unicode object on Python 2 and a str on Python 3. - self.path = upath(app_module.__path__[0]) if app_module is not None else None + self.path = upath(app_module.__path__[0]) def __repr__(self): return '' % self.label diff --git a/django/core/apps/cache.py b/django/core/apps/cache.py index 69f2f2c974..fb7bb6172e 100644 --- a/django/core/apps/cache.py +++ b/django/core/apps/cache.py @@ -1,6 +1,7 @@ "Utilities for loading models and the modules that contain them." from collections import defaultdict, OrderedDict +from contextlib import contextmanager from importlib import import_module import os import sys @@ -120,8 +121,7 @@ class AppCache(object): finally: self.nesting_level -= 1 - app_config = AppConfig( - name=app_name, app_module=app_module, models_module=models_module) + app_config = AppConfig(app_name, app_module, models_module) app_config.models = self.all_models[app_config.label] self.app_configs[app_config.label] = app_config @@ -257,7 +257,7 @@ class AppCache(object): self.populate() if only_installed: app_config = self.app_configs.get(app_label) - if app_config is None or not app_config.installed: + if app_config is None: return None if (self.available_apps is not None and app_config.name not in self.available_apps): @@ -304,6 +304,63 @@ class AppCache(object): def unset_available_apps(self): self.available_apps = None + ### DANGEROUS METHODS ### (only used to preserve existing tests) + + def _begin_with_app(self, app_name): + # Returns an opaque value that can be passed to _end_with_app(). + app_module = import_module(app_name) + models_module = import_module('%s.models' % app_name) + app_config = AppConfig(app_name, app_module, models_module) + if app_config.label in self.app_configs: + return None + else: + app_config.models = self.all_models[app_config.label] + self.app_configs[app_config.label] = app_config + return app_config + + def _end_with_app(self, app_config): + if app_config is not None: + del self.app_configs[app_config.label] + + @contextmanager + def _with_app(self, app_name): + app_config = self._begin_with_app(app_name) + try: + yield + finally: + self._end_with_app(app_config) + + def _begin_without_app(self, app_name): + # Returns an opaque value that can be passed to _end_without_app(). + return self.app_configs.pop(app_name.rpartition(".")[2], None) + + def _end_without_app(self, app_config): + if app_config is not None: + self.app_configs[app_config.label] = app_config + + @contextmanager + def _without_app(self, app_name): + app_config = self._begin_without_app(app_name) + try: + yield + finally: + self._end_without_app(app_config) + + def _begin_empty(self): + app_configs, self.app_configs = self.app_configs, OrderedDict() + return app_configs + + def _end_empty(self, app_configs): + self.app_configs = app_configs + + @contextmanager + def _empty(self): + app_configs = self._begin_empty() + try: + yield + finally: + self._end_empty(app_configs) + ### DEPRECATED METHODS GO BELOW THIS LINE ### def get_app(self, app_label): diff --git a/django/db/models/options.py b/django/db/models/options.py index cd68d5eae9..f7be089539 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -94,11 +94,11 @@ class Options(object): @property def app_config(self): # Don't go through get_app_config to avoid triggering populate(). - return self.app_cache.app_configs[self.app_label] + return self.app_cache.app_configs.get(self.app_label) @property def installed(self): - return self.app_config.installed + return self.app_config is not None def contribute_to_class(self, cls, name): from django.db import connection diff --git a/tests/admin_docs/tests.py b/tests/admin_docs/tests.py index 047bf920a2..66c125490f 100644 --- a/tests/admin_docs/tests.py +++ b/tests/admin_docs/tests.py @@ -4,6 +4,7 @@ from django.conf import settings from django.contrib.sites.models import Site from django.contrib.admindocs import utils from django.contrib.auth.models import User +from django.core.apps import app_cache from django.core.urlresolvers import reverse from django.test import TestCase from django.test.utils import override_settings @@ -13,27 +14,18 @@ class MiscTests(TestCase): urls = 'admin_docs.urls' def setUp(self): - self._old_installed = Site._meta.app_config.installed User.objects.create_superuser('super', None, 'secret') self.client.login(username='super', password='secret') - def tearDown(self): - Site._meta.app_config.installed = self._old_installed - - @override_settings( - SITE_ID=None, - INSTALLED_APPS=[app for app in settings.INSTALLED_APPS - if app != 'django.contrib.sites'], - ) def test_no_sites_framework(self): """ Without the sites framework, should not access SITE_ID or Site objects. Deleting settings is fine here as UserSettingsHolder is used. """ - Site._meta.app_config.installed = False - Site.objects.all().delete() - del settings.SITE_ID - self.client.get('/admindocs/views/') # should not raise + with self.settings(SITE_ID=None), app_cache._without_app('django.contrib.sites'): + Site.objects.all().delete() + del settings.SITE_ID + self.client.get('/admindocs/views/') # should not raise @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))