Fixed #27626 -- Moved MEDIA_URL/STATIC_URL validation to a system check.

6 files changed, 53 insertions, 93 deletions

diff --git a/django/conf/__init__.py b/django/conf/__init__.py
index e9611aa3fe..4fec2ee646 100644
--- a/django/conf/__init__.py
+++ b/django/conf/__init__.py
@@ -74,17 +74,7 @@ class LazySettings(LazyObject):
         return self._wrapped is not empty
 
 
-class BaseSettings(object):
-    """
-    Common logic for settings whether set by a module or by the user.
-    """
-    def __setattr__(self, name, value):
-        if name in ("MEDIA_URL", "STATIC_URL") and value and not value.endswith('/'):
-            raise ImproperlyConfigured("If set, %s must end with a slash" % name)
-        object.__setattr__(self, name, value)
-
-
-class Settings(BaseSettings):
+class Settings(object):
     def __init__(self, settings_module):
         # update this dict from global settings (but only for ALL_CAPS settings)
         for setting in dir(global_settings):
@@ -137,7 +127,7 @@ class Settings(BaseSettings):
         }
 
 
-class UserSettingsHolder(BaseSettings):
+class UserSettingsHolder(object):
     """
     Holder for user configured settings.
     """
diff --git a/django/core/checks/urls.py b/django/core/checks/urls.py
index 8187a4e561..e950cb5637 100644
--- a/django/core/checks/urls.py
+++ b/django/core/checks/urls.py
@@ -90,3 +90,20 @@ def get_warning_for_invalid_pattern(pattern):
         hint=hint,
         id="urls.E004",
     )]
+
+
+@register(Tags.urls)
+def check_url_settings(app_configs, **kwargs):
+    errors = []
+    for name in ('STATIC_URL', 'MEDIA_URL'):
+        value = getattr(settings, name)
+        if value and not value.endswith('/'):
+            errors.append(E006(name))
+    return errors
+
+
+def E006(name):
+    return Error(
+        'The {} setting must end with a slash.'.format(name),
+        id='urls.E006',
+    )
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index ef6bbcee0e..f8e5522a2b 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -675,3 +675,5 @@ The following checks are performed on your URL configuration:
   ``urlpatterns`` is a list of :func:`~django.conf.urls.url()` instances.
 * **urls.W005**: URL namespace ``<namespace>`` isn't unique. You may not be
   able to reverse all URLs in this namespace.
+* **urls.E006**: The :setting:`MEDIA_URL`/ :setting:`STATIC_URL` setting must
+  end with a slash.
diff --git a/tests/admin_scripts/tests.py b/tests/admin_scripts/tests.py
index 656736be82..af3ada5bcc 100644
--- a/tests/admin_scripts/tests.py
+++ b/tests/admin_scripts/tests.py
@@ -1144,7 +1144,11 @@ class ManageSettingsWithSettingsErrors(AdminScriptTestCase):
         Test listing available commands output note when only core commands are
         available.
         """
-        self.write_settings('settings.py', sdict={'MEDIA_URL': '"/no_ending_slash"'})
+        self.write_settings(
+            'settings.py',
+            extra='from django.core.exceptions import ImproperlyConfigured\n'
+                  'raise ImproperlyConfigured()',
+        )
         args = ['help']
         out, err = self.run_manage(args)
         self.assertOutput(out, 'only Django core commands are listed')
diff --git a/tests/check_framework/test_urls.py b/tests/check_framework/test_urls.py
index 3a52f21d59..b60578cbdb 100644
--- a/tests/check_framework/test_urls.py
+++ b/tests/check_framework/test_urls.py
@@ -1,14 +1,14 @@
 from django.conf import settings
 from django.core.checks.messages import Warning
 from django.core.checks.urls import (
-    check_url_config, check_url_namespaces_unique,
+    E006, check_url_config, check_url_namespaces_unique, check_url_settings,
     get_warning_for_invalid_pattern,
 )
 from django.test import SimpleTestCase
 from django.test.utils import override_settings
 
 
-class CheckUrlsTest(SimpleTestCase):
+class CheckUrlConfigTests(SimpleTestCase):
     @override_settings(ROOT_URLCONF='check_framework.urls.no_warnings')
     def test_no_warnings(self):
         result = check_url_config(None)
@@ -116,3 +116,28 @@ class CheckUrlsTest(SimpleTestCase):
     def test_check_unique_namespaces(self):
         result = check_url_namespaces_unique(None)
         self.assertEqual(result, [])
+
+
+class CheckURLSettingsTests(SimpleTestCase):
+
+    @override_settings(STATIC_URL='a/', MEDIA_URL='b/')
+    def test_slash_no_errors(self):
+        self.assertEqual(check_url_settings(None), [])
+
+    @override_settings(STATIC_URL='', MEDIA_URL='')
+    def test_empty_string_no_errors(self):
+        self.assertEqual(check_url_settings(None), [])
+
+    @override_settings(STATIC_URL='noslash')
+    def test_static_url_no_slash(self):
+        self.assertEqual(check_url_settings(None), [E006('STATIC_URL')])
+
+    @override_settings(STATIC_URL='slashes//')
+    def test_static_url_double_slash_allowed(self):
+        # The check allows for a double slash, presuming the user knows what
+        # they are doing.
+        self.assertEqual(check_url_settings(None), [])
+
+    @override_settings(MEDIA_URL='noslash')
+    def test_media_url_no_slash(self):
+        self.assertEqual(check_url_settings(None), [E006('MEDIA_URL')])
diff --git a/tests/settings_tests/tests.py b/tests/settings_tests/tests.py
index 8f50a854e4..bf015affc2 100644
--- a/tests/settings_tests/tests.py
+++ b/tests/settings_tests/tests.py
@@ -310,84 +310,6 @@ class TestComplexSettingOverride(SimpleTestCase):
             self.assertEqual(str(w[0].message), 'Overriding setting TEST_WARN can lead to unexpected behavior.')
 
 
-class TrailingSlashURLTests(SimpleTestCase):
-    """
-    Tests for the MEDIA_URL and STATIC_URL settings.
-
-    They must end with a slash to ensure there's a deterministic way to build
-    paths in templates.
-    """
-    settings_module = settings
-
-    def setUp(self):
-        self._original_media_url = self.settings_module.MEDIA_URL
-        self._original_static_url = self.settings_module.STATIC_URL
-
-    def tearDown(self):
-        self.settings_module.MEDIA_URL = self._original_media_url
-        self.settings_module.STATIC_URL = self._original_static_url
-
-    def test_blank(self):
-        """
-        The empty string is accepted, even though it doesn't end in a slash.
-        """
-        self.settings_module.MEDIA_URL = ''
-        self.assertEqual('', self.settings_module.MEDIA_URL)
-
-        self.settings_module.STATIC_URL = ''
-        self.assertEqual('', self.settings_module.STATIC_URL)
-
-    def test_end_slash(self):
-        """
-        It works if the value ends in a slash.
-        """
-        self.settings_module.MEDIA_URL = '/foo/'
-        self.assertEqual('/foo/', self.settings_module.MEDIA_URL)
-
-        self.settings_module.MEDIA_URL = 'http://media.foo.com/'
-        self.assertEqual('http://media.foo.com/', self.settings_module.MEDIA_URL)
-
-        self.settings_module.STATIC_URL = '/foo/'
-        self.assertEqual('/foo/', self.settings_module.STATIC_URL)
-
-        self.settings_module.STATIC_URL = 'http://static.foo.com/'
-        self.assertEqual('http://static.foo.com/', self.settings_module.STATIC_URL)
-
-    def test_no_end_slash(self):
-        """
-        An ImproperlyConfigured exception is raised if the value doesn't end
-        in a slash.
-        """
-        with self.assertRaises(ImproperlyConfigured):
-            self.settings_module.MEDIA_URL = '/foo'
-
-        with self.assertRaises(ImproperlyConfigured):
-            self.settings_module.MEDIA_URL = 'http://media.foo.com'
-
-        with self.assertRaises(ImproperlyConfigured):
-            self.settings_module.STATIC_URL = '/foo'
-
-        with self.assertRaises(ImproperlyConfigured):
-            self.settings_module.STATIC_URL = 'http://static.foo.com'
-
-    def test_double_slash(self):
-        """
-        If the value ends in more than one slash, presume they know what
-        they're doing.
-        """
-        self.settings_module.MEDIA_URL = '/wrong//'
-        self.assertEqual('/wrong//', self.settings_module.MEDIA_URL)
-
-        self.settings_module.MEDIA_URL = 'http://media.foo.com/wrong//'
-        self.assertEqual('http://media.foo.com/wrong//', self.settings_module.MEDIA_URL)
-
-        self.settings_module.STATIC_URL = '/wrong//'
-        self.assertEqual('/wrong//', self.settings_module.STATIC_URL)
-
-        self.settings_module.STATIC_URL = 'http://static.foo.com/wrong//'
-        self.assertEqual('http://static.foo.com/wrong//', self.settings_module.STATIC_URL)
-
-
 class SecureProxySslHeaderTest(SimpleTestCase):
 
     @override_settings(SECURE_PROXY_SSL_HEADER=None)