Skip to content

Commit

Permalink
Removed the app_config.installed flag.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aaugustin committed Dec 22, 2013
1 parent 972babc commit 9b3389b
Show file tree
Hide file tree
Showing 10 changed files with 99 additions and 60 deletions.
23 changes: 11 additions & 12 deletions django/contrib/contenttypes/tests.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -54,11 +55,9 @@ def get_absolute_url(self):
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):
Expand Down Expand Up @@ -223,15 +222,15 @@ def test_shortcut_view(self):
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):
"""
Expand Down
6 changes: 3 additions & 3 deletions django/contrib/gis/tests/geoapp/test_feeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
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:
Expand All @@ -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."
Expand Down
6 changes: 3 additions & 3 deletions django/contrib/gis/tests/geoapp/test_sitemaps.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
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
Expand All @@ -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):
Expand Down
4 changes: 0 additions & 4 deletions django/contrib/sitemaps/tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 7 additions & 8 deletions django/contrib/sitemaps/tests/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
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
Expand Down Expand Up @@ -108,15 +109,14 @@ def test_localized_priority(self):
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 = """<?xml version="1.0" encoding="UTF-8"?>
with app_cache._without_app('django.contrib.sites'):
response = self.client.get('/simple/sitemap.xml')
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
<url><loc>http://testserver/location/</loc><lastmod>%s</lastmod><changefreq>never</changefreq><priority>0.5</priority></url>
</urlset>
""" % 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.")
Expand All @@ -134,9 +134,8 @@ def test_sitemap_get_urls_no_site_2(self):
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):
"""
Expand Down
14 changes: 7 additions & 7 deletions django/contrib/sites/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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
Expand All @@ -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
Expand Down Expand Up @@ -67,10 +67,10 @@ def test_get_current_site(self):
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
Expand Down
6 changes: 1 addition & 5 deletions django/core/apps/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,10 @@ def __init__(self, name, app_module, models_module):
# 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 '<AppConfig: %s>' % self.label
63 changes: 60 additions & 3 deletions django/core/apps/cache.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -120,8 +121,7 @@ def load_app(self, app_name, can_postpone=False):
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

Expand Down Expand Up @@ -257,7 +257,7 @@ def get_model(self, app_label, model_name, only_installed=True):
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):
Expand Down Expand Up @@ -304,6 +304,63 @@ def set_available_apps(self, available):
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):
Expand Down
4 changes: 2 additions & 2 deletions django/db/models/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ def __init__(self, meta, app_label=None):
@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
Expand Down
18 changes: 5 additions & 13 deletions tests/admin_docs/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
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
Expand All @@ -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',))
Expand Down

0 comments on commit 9b3389b

Please sign in to comment.