From 9de9c240178bd9f61f55dad07e76ea458f0291f5 Mon Sep 17 00:00:00 2001
From: Abhaya Agarwal <abhaya@instascribe.com>
Date: Sun, 3 May 2015 09:40:24 +0530
Subject: [PATCH] Fixed #24105 -- Called Storage.get_valid_name() when
upload_to is callable
---
django/db/models/fields/files.py | 9 +++++++--
docs/howto/custom-file-storage.txt | 12 +++++++++---
docs/releases/1.9.txt | 4 +++-
tests/file_storage/models.py | 10 ++++++++++
tests/file_storage/tests.py | 10 ++++++++++
5 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py
index ba79e927dd..2363088a6e 100644
--- a/django/db/models/fields/files.py
+++ b/django/db/models/fields/files.py
@@ -244,8 +244,6 @@ class FileField(Field):
self.storage = storage or default_storage
self.upload_to = upload_to
- if callable(upload_to):
- self.generate_filename = upload_to
kwargs['max_length'] = kwargs.get('max_length', 100)
super(FileField, self).__init__(verbose_name, name, **kwargs)
@@ -326,6 +324,13 @@ class FileField(Field):
return os.path.normpath(self.storage.get_valid_name(os.path.basename(filename)))
def generate_filename(self, instance, filename):
+ # If upload_to is a callable, make sure that the path it returns is
+ # passed through get_valid_name() of the underlying storage.
+ if callable(self.upload_to):
+ directory_name, filename = os.path.split(self.upload_to(instance, filename))
+ filename = self.storage.get_valid_name(filename)
+ return os.path.normpath(os.path.join(directory_name, filename))
+
return os.path.join(self.get_directory_name(), self.get_filename(filename))
def save_form_data(self, instance, data):
diff --git a/docs/howto/custom-file-storage.txt b/docs/howto/custom-file-storage.txt
index a6c8745462..5140911591 100644
--- a/docs/howto/custom-file-storage.txt
+++ b/docs/howto/custom-file-storage.txt
@@ -88,9 +88,15 @@ instead).
.. method:: get_valid_name(name)
Returns a filename suitable for use with the underlying storage system. The
-``name`` argument passed to this method is the original filename sent to the
-server, after having any path information removed. Override this to customize
-how non-standard characters are converted to safe filenames.
+``name`` argument passed to this method is either the original filename sent to
+the server or, if ``upload_to`` is a callable, the filename returned by that
+method after any path information is removed. Override this to customize how
+non-standard characters are converted to safe filenames.
+
+.. versionchanged:: 1.9
+
+ In older versions, this method was not called when ``upload_to`` was a
+ callable.
The code provided on ``Storage`` retains only alpha-numeric characters, periods
and underscores from the original filename, removing everything else.
diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt
index 25868dafd3..473e64f95d 100644
--- a/docs/releases/1.9.txt
+++ b/docs/releases/1.9.txt
@@ -141,7 +141,9 @@ Email
File Storage
^^^^^^^^^^^^
-* ...
+* :meth:`Storage.get_valid_name()
+ <django.core.files.storage.Storage.get_valid_name>` is now called when
+ the :attr:`~django.db.models.FileField.upload_to` is a callable.
File Uploads
^^^^^^^^^^^^
diff --git a/tests/file_storage/models.py b/tests/file_storage/models.py
index c7523bdea4..9ab8dfbfe2 100644
--- a/tests/file_storage/models.py
+++ b/tests/file_storage/models.py
@@ -25,6 +25,12 @@ class OldStyleFSStorage(FileSystemStorage):
return super(OldStyleFSStorage, self).save(name, content)
+class CustomValidNameStorage(FileSystemStorage):
+ def get_valid_name(self, name):
+ # mark the name to show that this was called
+ return name + '_valid'
+
+
temp_storage_location = tempfile.mkdtemp()
temp_storage = FileSystemStorage(location=temp_storage_location)
@@ -41,6 +47,10 @@ class Storage(models.Model):
normal = models.FileField(storage=temp_storage, upload_to='tests')
custom = models.FileField(storage=temp_storage, upload_to=custom_upload_to)
random = models.FileField(storage=temp_storage, upload_to=random_upload_to)
+ custom_valid_name = models.FileField(
+ storage=CustomValidNameStorage(location=temp_storage_location),
+ upload_to=random_upload_to,
+ )
default = models.FileField(storage=temp_storage, upload_to='tests', default='tests/default.txt')
empty = models.FileField(storage=temp_storage)
limited_length = models.FileField(storage=temp_storage, upload_to='tests', max_length=20)
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
index 2c08a5312f..2b41b5abd8 100644
--- a/tests/file_storage/tests.py
+++ b/tests/file_storage/tests.py
@@ -597,6 +597,16 @@ class FileFieldStorageTests(SimpleTestCase):
self.assertTrue(obj.random.name.endswith("/random_file"))
obj.random.close()
+ def test_custom_valid_name_callable_upload_to(self):
+ """
+ Storage.get_valid_name() should be called when upload_to is a callable.
+ """
+ obj = Storage()
+ obj.custom_valid_name.save("random_file", ContentFile("random content"))
+ # CustomValidNameStorage.get_valid_name() appends '_valid' to the name
+ self.assertTrue(obj.custom_valid_name.name.endswith("/random_file_valid"))
+ obj.custom_valid_name.close()
+
def test_filefield_pickling(self):
# Push an object into the cache to make sure it pickles properly
obj = Storage()