Fixed #24105 -- Called Storage.get_valid_name() when upload_to is callable

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):

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.

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
 ^^^^^^^^^^^^

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)

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()