Fixed #26038 -- Changed FileSystemStorage defaults on setting change.

Thanks to Dave Voutila for the report and Tim for the review.

3 files changed, 84 insertions, 28 deletions

diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index efd8fc13b7..208884e1cf 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -6,11 +6,12 @@ from django.conf import settings
 from django.core.exceptions import SuspiciousFileOperation
 from django.core.files import File, locks
 from django.core.files.move import file_move_safe
+from django.core.signals import setting_changed
 from django.utils._os import abspathu, safe_join
 from django.utils.crypto import get_random_string
 from django.utils.deconstruct import deconstructible
 from django.utils.encoding import filepath_to_uri, force_text
-from django.utils.functional import LazyObject
+from django.utils.functional import LazyObject, cached_property
 from django.utils.module_loading import import_string
 from django.utils.six.moves.urllib.parse import urljoin
 from django.utils.text import get_valid_filename
@@ -165,24 +166,49 @@ class FileSystemStorage(Storage):
     """
 
     def __init__(self, location=None, base_url=None, file_permissions_mode=None,
-            directory_permissions_mode=None):
-        if location is None:
-            location = settings.MEDIA_ROOT
-        self.base_location = location
-        self.location = abspathu(self.base_location)
-        if base_url is None:
-            base_url = settings.MEDIA_URL
-        elif not base_url.endswith('/'):
+                 directory_permissions_mode=None):
+        self._location = location
+        if base_url is not None and not base_url.endswith('/'):
             base_url += '/'
-        self.base_url = base_url
-        self.file_permissions_mode = (
-            file_permissions_mode if file_permissions_mode is not None
-            else settings.FILE_UPLOAD_PERMISSIONS
-        )
-        self.directory_permissions_mode = (
-            directory_permissions_mode if directory_permissions_mode is not None
-            else settings.FILE_UPLOAD_DIRECTORY_PERMISSIONS
-        )
+        self._base_url = base_url
+        self._file_permissions_mode = file_permissions_mode
+        self._directory_permissions_mode = directory_permissions_mode
+        setting_changed.connect(self._clear_cached_properties)
+
+    def _clear_cached_properties(self, setting, **kwargs):
+        """Reset setting based property values."""
+        if setting == 'MEDIA_ROOT':
+            self.__dict__.pop('base_location', None)
+            self.__dict__.pop('location', None)
+        elif setting == 'MEDIA_URL':
+            self.__dict__.pop('base_url', None)
+        elif setting == 'FILE_UPLOAD_PERMISSIONS':
+            self.__dict__.pop('file_permissions_mode', None)
+        elif setting == 'FILE_UPLOAD_DIRECTORY_PERMISSIONS':
+            self.__dict__.pop('directory_permissions_mode', None)
+
+    def _value_or_setting(self, value, setting):
+        return setting if value is None else value
+
+    @cached_property
+    def base_location(self):
+        return self._value_or_setting(self._location, settings.MEDIA_ROOT)
+
+    @cached_property
+    def location(self):
+        return abspathu(self.base_location)
+
+    @cached_property
+    def base_url(self):
+        return self._value_or_setting(self._base_url, settings.MEDIA_URL)
+
+    @cached_property
+    def file_permissions_mode(self):
+        return self._value_or_setting(self._file_permissions_mode, settings.FILE_UPLOAD_PERMISSIONS)
+
+    @cached_property
+    def directory_permissions_mode(self):
+        return self._value_or_setting(self._directory_permissions_mode, settings.FILE_UPLOAD_DIRECTORY_PERMISSIONS)
 
     def _open(self, name, mode='rb'):
         return File(open(self.path(name), mode))
diff --git a/django/test/signals.py b/django/test/signals.py
index a647e2895b..1a86b1f846 100644
--- a/django/test/signals.py
+++ b/django/test/signals.py
@@ -120,15 +120,7 @@ def language_changed(**kwargs):
 
 @receiver(setting_changed)
 def file_storage_changed(**kwargs):
-    file_storage_settings = {
-        'DEFAULT_FILE_STORAGE',
-        'FILE_UPLOAD_DIRECTORY_PERMISSIONS',
-        'FILE_UPLOAD_PERMISSIONS',
-        'MEDIA_ROOT',
-        'MEDIA_URL',
-    }
-
-    if kwargs['setting'] in file_storage_settings:
+    if kwargs['setting'] == 'DEFAULT_FILE_STORAGE':
         from django.core.files.storage import default_storage
         default_storage._wrapped = empty
 
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
index 6b02073bbf..25c6161d5e 100644
--- a/tests/file_storage/tests.py
+++ b/tests/file_storage/tests.py
@@ -83,7 +83,7 @@ class FileStorageDeconstructionTests(unittest.TestCase):
         self.assertEqual(kwargs, kwargs_orig)
 
 
-class FileStorageTests(unittest.TestCase):
+class FileStorageTests(SimpleTestCase):
     storage_class = FileSystemStorage
 
     def setUp(self):
@@ -403,6 +403,44 @@ class FileStorageTests(unittest.TestCase):
         with self.assertRaises(AssertionError):
             self.storage.delete('')
 
+    @override_settings(
+        MEDIA_ROOT='media_root',
+        MEDIA_URL='media_url/',
+        FILE_UPLOAD_PERMISSIONS=0o777,
+        FILE_UPLOAD_DIRECTORY_PERMISSIONS=0o777,
+    )
+    def test_setting_changed(self):
+        """
+        Properties using settings values as defaults should be updated on
+        referenced settings change while specified values should be unchanged.
+        """
+        storage = self.storage_class(
+            location='explicit_location',
+            base_url='explicit_base_url/',
+            file_permissions_mode=0o666,
+            directory_permissions_mode=0o666,
+        )
+        defaults_storage = self.storage_class()
+        settings = {
+            'MEDIA_ROOT': 'overriden_media_root',
+            'MEDIA_URL': 'overriden_media_url/',
+            'FILE_UPLOAD_PERMISSIONS': 0o333,
+            'FILE_UPLOAD_DIRECTORY_PERMISSIONS': 0o333,
+        }
+        with self.settings(**settings):
+            self.assertEqual(storage.base_location, 'explicit_location')
+            self.assertIn('explicit_location', storage.location)
+            self.assertEqual(storage.base_url, 'explicit_base_url/')
+            self.assertEqual(storage.file_permissions_mode, 0o666)
+            self.assertEqual(storage.directory_permissions_mode, 0o666)
+            self.assertEqual(defaults_storage.base_location, settings['MEDIA_ROOT'])
+            self.assertIn(settings['MEDIA_ROOT'], defaults_storage.location)
+            self.assertEqual(defaults_storage.base_url, settings['MEDIA_URL'])
+            self.assertEqual(defaults_storage.file_permissions_mode, settings['FILE_UPLOAD_PERMISSIONS'])
+            self.assertEqual(
+                defaults_storage.directory_permissions_mode, settings['FILE_UPLOAD_DIRECTORY_PERMISSIONS']
+            )
+
 
 class CustomStorage(FileSystemStorage):
     def get_available_name(self, name, max_length=None):