File delegate-os-path-filename-generation-to-storage.patch of Package python-Django
commit c8ab1ee37ce5e486b9c59c7f8f03999457c67d86
Author: Cristiano <cristianocca@hotmail.com>
Date: Sun Mar 20 22:51:17 2016 -0300
Fixed #26058 -- Delegated os.path bits of FileField's filename generation to the Storage.
(cherry picked from commit 914c72be2abb1c6dd860cb9279beaa66409ae1b2)
diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 39cf5d800b85..df92f0af8a28 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -60,10 +60,7 @@ class Storage(object):
)
name = self.get_available_name(name)
- name = self._save(name, content)
-
- # Store filenames with forward slashes, even on Windows
- return force_text(name.replace('\\', '/'))
+ return self._save(name, content)
# These methods are part of the public API, with default implementations.
@@ -105,6 +102,15 @@ class Storage(object):
name = os.path.join(dir_name, "%s_%s%s" % (file_root, get_random_string(7), file_ext))
return name
+ def generate_filename(self, filename):
+ """
+ Validate the filename by calling get_valid_name() and return a filename
+ to be passed to the save() method.
+ """
+ # `filename` may include a path as returned by FileField.upload_to.
+ dirname, filename = os.path.split(filename)
+ return os.path.normpath(os.path.join(dirname, self.get_valid_name(filename)))
+
def path(self, name):
"""
Returns a local filesystem path where the file can be retrieved using
@@ -274,7 +280,8 @@ class FileSystemStorage(Storage):
if self.file_permissions_mode is not None:
os.chmod(full_path, self.file_permissions_mode)
- return name
+ # Store filenames with forward slashes, even on Windows.
+ return force_text(name.replace('\\', '/'))
def delete(self, name):
assert name, "The name argument is not allowed to be empty."
diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py
index bf447591b07b..20374ba8e69a 100644
--- a/django/db/models/fields/files.py
+++ b/django/db/models/fields/files.py
@@ -1,5 +1,6 @@
import datetime
import os
+import posixpath
import warnings
from django import forms
@@ -11,6 +12,7 @@ from django.db.models import signals
from django.db.models.fields import Field
from django.utils import six
from django.utils.deprecation import RemovedInDjango110Warning
+from django.utils.deprecation import RemovedInDjango20Warning
from django.utils.encoding import force_str, force_text
from django.utils.inspect import func_supports_parameter
from django.utils.translation import ugettext_lazy as _
@@ -319,13 +321,34 @@ class FileField(Field):
setattr(cls, self.name, self.descriptor_class(self))
def get_directory_name(self):
+ warnings.warn(
+ 'FileField now delegates file name and folder processing to the '
+ 'storage. get_directory_name() will be removed in Django 2.0.',
+ RemovedInDjango20Warning, stacklevel=2
+ )
return os.path.normpath(force_text(datetime.datetime.now().strftime(force_str(self.upload_to))))
def get_filename(self, filename):
+ warnings.warn(
+ 'FileField now delegates file name and folder processing to the '
+ 'storage. get_filename() will be removed in Django 2.0.',
+ RemovedInDjango20Warning, stacklevel=2
+ )
return os.path.normpath(self.storage.get_valid_name(os.path.basename(filename)))
def generate_filename(self, instance, filename):
- return os.path.join(self.get_directory_name(), self.get_filename(filename))
+ """
+ Apply (if callable) or prepend (if a string) upload_to to the filename,
+ then delegate further processing of the name to the storage backend.
+ Until the storage layer, all file paths are expected to be Unix style
+ (with forward slashes).
+ """
+ if callable(self.upload_to):
+ filename = self.upload_to(instance, filename)
+ else:
+ dirname = force_text(datetime.datetime.now().strftime(force_str(self.upload_to)))
+ filename = posixpath.join(dirname, filename)
+ return self.storage.generate_filename(filename)
def save_form_data(self, instance, data):
# Important: None means "no change", other false value means "clear"
diff --git a/docs/ref/files/storage.txt b/docs/ref/files/storage.txt
index 976958dfd5cd..6cd6a6268bdd 100644
--- a/docs/ref/files/storage.txt
+++ b/docs/ref/files/storage.txt
@@ -147,6 +147,21 @@ The Storage Class
Returns a filename based on the ``name`` parameter that's suitable
for use on the target storage system.
+ .. method:: generate_filename(filename)
+
+ .. versionadded:: 1.10
+
+ Validates the ``filename`` by calling :attr:`get_valid_name()` and
+ returns a filename to be passed to the :meth:`save` method.
+
+ The ``filename`` argument may include a path as returned by
+ :attr:`FileField.upload_to <django.db.models.FileField.upload_to>`.
+ In that case, the path won't be passed to :attr:`get_valid_name()` but
+ will be prepended back to the resulting name.
+
+ The default implementation uses :mod:`os.path` operations. Override
+ this method if that's not appropriate for your storage.
+
.. method:: listdir(path)
Lists the contents of the specified path, returning a 2-tuple of lists;
diff --git a/tests/file_storage/test_generate_filename.py b/tests/file_storage/test_generate_filename.py
new file mode 100644
index 000000000000..44320138509b
--- /dev/null
+++ b/tests/file_storage/test_generate_filename.py
@@ -0,0 +1,117 @@
+import os
+import warnings
+
+from django.core.files.base import ContentFile
+from django.core.files.storage import Storage
+from django.db.models import FileField
+from django.test import SimpleTestCase
+
+
+class AWSS3Storage(Storage):
+ """
+ Simulate an AWS S3 storage which uses Unix-like paths and allows any
+ characters in file names but where there aren't actual folders but just
+ keys.
+ """
+ prefix = 'mys3folder/'
+
+ def _save(self, name, content):
+ """
+ This method is important to test that Storage.save() doesn't replace
+ '\' with '/' (rather FileSystemStorage.save() does).
+ """
+ return name
+
+ def get_valid_name(self, name):
+ return name
+
+ def get_available_name(self, name, max_length=None):
+ return name
+
+ def generate_filename(self, filename):
+ """
+ This is the method that's important to override when using S3 so that
+ os.path() isn't called, which would break S3 keys.
+ """
+ return self.prefix + self.get_valid_name(filename)
+
+
+class GenerateFilenameStorageTests(SimpleTestCase):
+
+ def test_filefield_get_directory_deprecation(self):
+ with warnings.catch_warnings(record=True) as warns:
+ warnings.simplefilter('always')
+ f = FileField(upload_to='some/folder/')
+ self.assertEqual(f.get_directory_name(), os.path.normpath('some/folder/'))
+
+ self.assertEqual(len(warns), 1)
+ self.assertEqual(
+ warns[0].message.args[0],
+ 'FileField now delegates file name and folder processing to the '
+ 'storage. get_directory_name() will be removed in Django 2.0.'
+ )
+
+ def test_filefield_get_filename_deprecation(self):
+ with warnings.catch_warnings(record=True) as warns:
+ warnings.simplefilter('always')
+ f = FileField(upload_to='some/folder/')
+ self.assertEqual(f.get_filename('some/folder/test.txt'), 'test.txt')
+
+ self.assertEqual(len(warns), 1)
+ self.assertEqual(
+ warns[0].message.args[0],
+ 'FileField now delegates file name and folder processing to the '
+ 'storage. get_filename() will be removed in Django 2.0.'
+ )
+
+ def test_filefield_generate_filename(self):
+ f = FileField(upload_to='some/folder/')
+ self.assertEqual(
+ f.generate_filename(None, 'test with space.txt'),
+ os.path.normpath('some/folder/test_with_space.txt')
+ )
+
+ def test_filefield_generate_filename_with_upload_to(self):
+ def upload_to(instance, filename):
+ return 'some/folder/' + filename
+
+ f = FileField(upload_to=upload_to)
+ self.assertEqual(
+ f.generate_filename(None, 'test with space.txt'),
+ os.path.normpath('some/folder/test_with_space.txt')
+ )
+
+ def test_filefield_awss3_storage(self):
+ """
+ Simulate a FileField with an S3 storage which uses keys rather than
+ folders and names. FileField and Storage shouldn't have any os.path()
+ calls that break the key.
+ """
+ storage = AWSS3Storage()
+ folder = 'not/a/folder/'
+
+ f = FileField(upload_to=folder, storage=storage)
+ key = 'my-file-key\\with odd characters'
+ data = ContentFile('test')
+ expected_key = AWSS3Storage.prefix + folder + key
+
+ # Simulate call to f.save()
+ result_key = f.generate_filename(None, key)
+ self.assertEqual(result_key, expected_key)
+
+ result_key = storage.save(result_key, data)
+ self.assertEqual(result_key, expected_key)
+
+ # Repeat test with a callable.
+ def upload_to(instance, filename):
+ # Return a non-normalized path on purpose.
+ return folder + filename
+
+ f = FileField(upload_to=upload_to, storage=storage)
+
+ # Simulate call to f.save()
+ result_key = f.generate_filename(None, key)
+ self.assertEqual(result_key, expected_key)
+
+ result_key = storage.save(result_key, data)
+ self.assertEqual(result_key, expected_key)
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
index b489c025bf41..e8b7ac166cfe 100644
--- a/tests/file_storage/tests.py
+++ b/tests/file_storage/tests.py
@@ -599,7 +599,7 @@ class FileFieldStorageTests(TestCase):
# upload_to can be empty, meaning it does not use subdirectory.
obj = Storage()
obj.empty.save('django_test.txt', ContentFile('more content'))
- self.assertEqual(obj.empty.name, "./django_test.txt")
+ self.assertEqual(obj.empty.name, "django_test.txt")
self.assertEqual(obj.empty.read(), b"more content")
obj.empty.close()
diff --git a/tests/model_fields/test_imagefield.py b/tests/model_fields/test_imagefield.py
index c2908b06e4ac..8e9b60ac65ff 100644
--- a/tests/model_fields/test_imagefield.py
+++ b/tests/model_fields/test_imagefield.py
@@ -49,10 +49,10 @@ class ImageFieldTestMixin(object):
os.mkdir(temp_storage_dir)
file_path1 = os.path.join(os.path.dirname(upath(__file__)), "4x8.png")
- self.file1 = self.File(open(file_path1, 'rb'))
+ self.file1 = self.File(open(file_path1, 'rb'), name='4x8.png')
file_path2 = os.path.join(os.path.dirname(upath(__file__)), "8x4.png")
- self.file2 = self.File(open(file_path2, 'rb'))
+ self.file2 = self.File(open(file_path2, 'rb'), name='8x4.png')
def tearDown(self):
"""
