Ticket #10258: 10258-on-9160.diff

django/core/files/storage.py

@@ -1 +1 @@
 import os
 import errno
 import urlparse
+import itertools
 
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
@@ -65 +66 @@
         """
 
         dir_name, file_name = os.path.split(name)
-        # If the filename already exists, keep adding an underscore to the name
-        # of the file until the filename doesn't exist.
-        while self.exists(name):
+        # If the filename already exists, add a number to the filename
+        # until an available name is found.
+        if self.exists(name):
             try:
                 dot_index = file_name.rindex('.')
             except ValueError: # filename has no dot
-                file_name += '_'
+                template = "%s_%%d" % (file_name,)
             else:
-                file_name = file_name[:dot_index] + '_' + file_name[dot_index:]
-            name = os.path.join(dir_name, file_name)
+                template = "%s_%%d%s" % (file_name[:dot_index], file_name[dot_index:],)
+            # Iterate indefinitely until a filename is found.
+            count = itertools.count(1)
+            while self.exists(name):
+                name = os.path.join(dir_name, template % count.next())
         return name
 
     def path(self, name):

tests/modeltests/files/models.py

@@ -88 +88 @@
 >>> '-'.join(obj1.normal.chunks(chunk_size=2))
 'co-nt-en-t'
 
-# Save another file with the same name.
+# Save more files with the same name; check for _[x] appendix.
+
+>>> objs = [ Storage() for i in range(10) ]
+>>> count = 1
+>>> for o in objs: 
+...     o.normal.save('django_test.txt', ContentFile('Ten files, same content'))
+>>> map(lambda o: o.normal, objs)
+[<FieldFile: tests/django_test_1.txt>, <FieldFile: tests/django_test_2.txt>, <FieldFile: tests/django_test_3.txt>, <FieldFile: tests/django_test_4.txt>, <FieldFile: tests/django_test_5.txt>, <FieldFile: tests/django_test_6.txt>, <FieldFile: tests/django_test_7.txt>, <FieldFile: tests/django_test_8.txt>, <FieldFile: tests/django_test_9.txt>, <FieldFile: tests/django_test_10.txt>]
+>>> for o in objs: o.delete()
 
 >>> obj2 = Storage()
 >>> obj2.normal.save('django_test.txt', ContentFile('more content'))
->>> obj2.normal
-<FieldFile: tests/django_test_.txt>
 >>> obj2.normal.size
 12
 
@@ -102 +108 @@
 >>> cache.set('obj1', obj1)
 >>> cache.set('obj2', obj2)
 >>> cache.get('obj2').normal
-<FieldFile: tests/django_test_.txt>
+<FieldFile: tests/django_test_1.txt>
 
 # Deleting an object deletes the file it uses, if there are no other objects
 # still using that file.
@@ -110 +116 @@
 >>> obj2.delete()
 >>> obj2.normal.save('django_test.txt', ContentFile('more content'))
 >>> obj2.normal
-<FieldFile: tests/django_test_.txt>
+<FieldFile: tests/django_test_1.txt>
 
 # Default values allow an object to access a single file.
 

tests/regressiontests/file_storage/tests.py

@@ -121 +121 @@
         name = self.save_file('conflict')
         self.thread.join()
         self.assert_(self.storage.exists('conflict'))
-        self.assert_(self.storage.exists('conflict_'))
+        self.assert_(self.storage.exists('conflict_1'))
         self.storage.delete('conflict')
-        self.storage.delete('conflict_')
+        self.storage.delete('conflict_1')
 
 class FileStoragePermissions(TestCase):
     def setUp(self):
@@ -141 +141 @@
         actual_mode = os.stat(self.storage.path(name))[0] & 0777
         self.assertEqual(actual_mode, 0666)
 
-
 class FileStorageDirectoryNameParsing(TestCase):
     """Regression test for #9610: Using a dotted pathname would mangle
     the directory name instead of the filename if the file already
@@ -158 +157 @@
         self.storage.save('dotted.path/test', ContentFile("2"))
 
         self.assertFalse(os.path.exists(os.path.join(self.storage_dir, 'dotted_.path')))
-        self.assertTrue(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/test_')))
+        self.assertTrue(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/test_1')))