Commit 56c461a0 authored by Simon Charette's avatar Simon Charette
Browse files

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

Thanks to Dave Voutila for the report and Tim for the review.
parent 6ea7b677
Loading
Loading
Loading
Loading
+44 −18
Original line number Diff line number Diff line
@@ -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
@@ -166,23 +167,48 @@ 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('/'):
        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))
+1 −9
Original line number Diff line number Diff line
@@ -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

+39 −1
Original line number Diff line number Diff line
@@ -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):