From 0ec4dc91e0e7befdd06aa0613b5d0fbe3c785ee7 Mon Sep 17 00:00:00 2001 From: Ling-Xiao Yang Date: Mon, 2 Jan 2017 17:35:43 -0500 Subject: [PATCH] Fixed #27661 -- Moved FileSystemFinder's ImproperlyConfigured exceptions to system checks. Thanks Simon Charette, Mariusz Felisiak, Tim Graham, and Adam Johnson for review. --- django/contrib/staticfiles/apps.py | 5 ++ django/contrib/staticfiles/checks.py | 14 +++++ django/contrib/staticfiles/finders.py | 33 ++++++++--- docs/ref/checks.txt | 12 ++++ tests/staticfiles_tests/test_checks.py | 77 +++++++++++++++++++++++++ tests/staticfiles_tests/test_finders.py | 9 --- 6 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 django/contrib/staticfiles/checks.py create mode 100644 tests/staticfiles_tests/test_checks.py diff --git a/django/contrib/staticfiles/apps.py b/django/contrib/staticfiles/apps.py index 0911c0113d..65865da0d2 100644 --- a/django/contrib/staticfiles/apps.py +++ b/django/contrib/staticfiles/apps.py @@ -1,4 +1,6 @@ from django.apps import AppConfig +from django.contrib.staticfiles.checks import check_finders +from django.core import checks from django.utils.translation import ugettext_lazy as _ @@ -6,3 +8,6 @@ class StaticFilesConfig(AppConfig): name = 'django.contrib.staticfiles' verbose_name = _("Static Files") ignore_patterns = ['CVS', '.*', '*~'] + + def ready(self): + checks.register(check_finders, 'staticfiles') diff --git a/django/contrib/staticfiles/checks.py b/django/contrib/staticfiles/checks.py new file mode 100644 index 0000000000..fb57bf726d --- /dev/null +++ b/django/contrib/staticfiles/checks.py @@ -0,0 +1,14 @@ +from django.contrib.staticfiles.finders import get_finders + + +def check_finders(app_configs=None, **kwargs): + """Check all registered staticfiles finders.""" + errors = [] + for finder in get_finders(): + try: + finder_errors = finder.check() + except NotImplementedError: + pass + else: + errors.extend(finder_errors) + return errors diff --git a/django/contrib/staticfiles/finders.py b/django/contrib/staticfiles/finders.py index fcb31c8547..76029b33b6 100644 --- a/django/contrib/staticfiles/finders.py +++ b/django/contrib/staticfiles/finders.py @@ -5,6 +5,7 @@ from collections import OrderedDict from django.apps import apps from django.conf import settings from django.contrib.staticfiles import utils +from django.core.checks import Error from django.core.exceptions import ImproperlyConfigured from django.core.files.storage import ( FileSystemStorage, Storage, default_storage, @@ -21,6 +22,11 @@ class BaseFinder: """ A base file finder to be used for custom staticfiles finder classes. """ + def check(self, **kwargs): + raise NotImplementedError( + 'subclasses may provide a check() method to verify the finder is ' + 'configured correctly.' + ) def find(self, path, all=False): """ @@ -52,19 +58,11 @@ class FileSystemFinder(BaseFinder): self.locations = [] # Maps dir paths to an appropriate storage instance self.storages = OrderedDict() - if not isinstance(settings.STATICFILES_DIRS, (list, tuple)): - raise ImproperlyConfigured( - "Your STATICFILES_DIRS setting is not a tuple or list; " - "perhaps you forgot a trailing comma?") for root in settings.STATICFILES_DIRS: if isinstance(root, (list, tuple)): prefix, root = root else: prefix = '' - if settings.STATIC_ROOT and os.path.abspath(settings.STATIC_ROOT) == os.path.abspath(root): - raise ImproperlyConfigured( - "The STATICFILES_DIRS setting should " - "not contain the STATIC_ROOT setting") if (prefix, root) not in self.locations: self.locations.append((prefix, root)) for prefix, root in self.locations: @@ -73,6 +71,25 @@ class FileSystemFinder(BaseFinder): self.storages[root] = filesystem_storage super().__init__(*args, **kwargs) + def check(self, **kwargs): + errors = [] + if not isinstance(settings.STATICFILES_DIRS, (list, tuple)): + errors.append(Error( + 'The STATICFILES_DIRS setting is not a tuple or list.', + hint='Perhaps you forgot a trailing comma?', + id='staticfiles.E001', + )) + for root in settings.STATICFILES_DIRS: + if isinstance(root, (list, tuple)): + _, root = root + if settings.STATIC_ROOT and os.path.abspath(settings.STATIC_ROOT) == os.path.abspath(root): + errors.append(Error( + 'The STATICFILES_DIRS setting should not contain the ' + 'STATIC_ROOT setting.', + id='staticfiles.E002', + )) + return errors + def find(self, path, all=False): """ Looks for files in the extra locations diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 518ed507af..35c0bd6987 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -83,6 +83,7 @@ Django's system checks are organized using the following tags: * ``models``: Checks of model, field, and manager definitions. * ``security``: Checks security related configuration. * ``signals``: Checks on signal declarations and handler registrations. +* ``staticfiles``: Checks :mod:`django.contrib.staticfiles` configuration. * ``templates``: Checks template related configuration. * ``urls``: Checks URL configuration. @@ -675,3 +676,14 @@ The following checks are performed on any model using a ````. * **sites.E002**: ``CurrentSiteManager`` cannot use ```` as it is not a foreign key or a many-to-many field. + +``staticfiles`` +--------------- + +The following checks verify that :mod:`django.contrib.staticfiles` is correctly +configured: + +* **staticfiles.E001**: The :setting:`STATICFILES_DIRS` setting is not a tuple + or list. +* **staticfiles.E002**: The :setting:`STATICFILES_DIRS` setting should not + contain the :setting:`STATIC_ROOT` setting. diff --git a/tests/staticfiles_tests/test_checks.py b/tests/staticfiles_tests/test_checks.py new file mode 100644 index 0000000000..0fe432b5c7 --- /dev/null +++ b/tests/staticfiles_tests/test_checks.py @@ -0,0 +1,77 @@ +from unittest import mock + +from django.conf import settings +from django.contrib.staticfiles.checks import check_finders +from django.contrib.staticfiles.finders import BaseFinder +from django.core.checks import Error +from django.test import SimpleTestCase, override_settings + + +class FindersCheckTests(SimpleTestCase): + + def test_base_finder_check_not_implemented(self): + finder = BaseFinder() + msg = 'subclasses may provide a check() method to verify the finder is configured correctly.' + with self.assertRaisesMessage(NotImplementedError, msg): + finder.check() + + def test_check_finders(self): + """check_finders() concatenates all errors.""" + error1 = Error('1') + error2 = Error('2') + error3 = Error('3') + + def get_finders(): + class Finder1(BaseFinder): + def check(self, **kwargs): + return [error1] + + class Finder2(BaseFinder): + def check(self, **kwargs): + return [] + + class Finder3(BaseFinder): + def check(self, **kwargs): + return [error2, error3] + + class Finder4(BaseFinder): + pass + + return [Finder1(), Finder2(), Finder3(), Finder4()] + + with mock.patch('django.contrib.staticfiles.checks.get_finders', get_finders): + errors = check_finders(None) + self.assertEqual(errors, [error1, error2, error3]) + + def test_no_errors_with_test_settings(self): + self.assertEqual(check_finders(None), []) + + @override_settings(STATICFILES_DIRS='a string') + def test_dirs_not_tuple_or_list(self): + self.assertEqual(check_finders(None), [ + Error( + 'The STATICFILES_DIRS setting is not a tuple or list.', + hint='Perhaps you forgot a trailing comma?', + id='staticfiles.E001', + ) + ]) + + @override_settings(STATICFILES_DIRS=['/fake/path', settings.STATIC_ROOT]) + def test_dirs_contains_static_root(self): + self.assertEqual(check_finders(None), [ + Error( + 'The STATICFILES_DIRS setting should not contain the ' + 'STATIC_ROOT setting.', + id='staticfiles.E002', + ) + ]) + + @override_settings(STATICFILES_DIRS=[('prefix', settings.STATIC_ROOT)]) + def test_dirs_contains_static_root_in_tuple(self): + self.assertEqual(check_finders(None), [ + Error( + 'The STATICFILES_DIRS setting should not contain the ' + 'STATIC_ROOT setting.', + id='staticfiles.E002', + ) + ]) diff --git a/tests/staticfiles_tests/test_finders.py b/tests/staticfiles_tests/test_finders.py index 0b661cb103..20d3060092 100644 --- a/tests/staticfiles_tests/test_finders.py +++ b/tests/staticfiles_tests/test_finders.py @@ -103,15 +103,6 @@ class TestMiscFinder(SimpleTestCase): [os.path.join(TEST_ROOT, 'project', 'documents')] ) - @override_settings(STATICFILES_DIRS='a string') - def test_non_tuple_raises_exception(self): - """ - We can't determine if STATICFILES_DIRS is set correctly just by - looking at the type, but we can determine if it's definitely wrong. - """ - with self.assertRaises(ImproperlyConfigured): - finders.FileSystemFinder() - @override_settings(MEDIA_ROOT='') def test_location_empty(self): with self.assertRaises(ImproperlyConfigured):