Commit 5366aa96 authored by Jacob Kaplan-Moss's avatar Jacob Kaplan-Moss
Browse files

Fixed #10258: handle duplicate file names better. 

Instead of just continually appending "_" to duplicate file names, Django's
default storage now appends `_1`, `_2`, `_3`, etc.

Thanks to ianschenck and Thilo.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12552 bcc190cf-cafb-0310-a4f2-bffc1f526a37
parent 43b47a87

django/core/files/storage.py

+6 −4
Original line number Diff line number Diff line 
import os

import errno

import urlparse

import itertools



from django.conf import settings

from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
@@ -65,13 +66,14 @@ class Storage(object): 
        """

        dir_name, file_name = os.path.split(name)

        file_root, file_ext = os.path.splitext(file_name)

        # If the filename already exists, keep adding an underscore (before the

        # file extension, if one exists) to the filename until the generated

        # If the filename already exists, add an underscore and a number (before

        # the file extension, if one exists) to the filename until the generated

        # filename doesn't exist.

        count = itertools.count(1)

        while self.exists(name):

            file_root += '_'

            # file_ext includes the dot.

            name = os.path.join(dir_name, file_root + file_ext)

            name = os.path.join(dir_name, "%s_%s%s" % (file_root, count.next(), file_ext))



        return name



    def path(self, name):

docs/howto/custom-file-storage.txt

+2 −2
Original line number Diff line number Diff line
@@ -88,5 +88,5 @@ the provided filename into account. The ``name`` argument passed to this method 
will have already cleaned to a filename valid for the storage system, according

to the ``get_valid_name()`` method described above.



The code provided on ``Storage`` simply appends underscores to the filename

until it finds one that's available in the destination directory.
 
The code provided on ``Storage`` simply appends ``"_1"``, ``"_2"``, etc. to the

filename until it finds one that's available in the destination directory.

tests/modeltests/files/models.py

+17 −6
Original line number Diff line number Diff line
@@ -5,14 +5,11 @@ 
and where files should be stored.

"""



import shutil

import random

import tempfile

from django.db import models

from django.core.files.base import ContentFile

from django.core.files.uploadedfile import SimpleUploadedFile

from django.core.files.storage import FileSystemStorage

from django.core.cache import cache



temp_storage_location = tempfile.mkdtemp()

temp_storage = FileSystemStorage(location=temp_storage_location)
@@ -64,6 +61,7 @@ ValueError: The 'normal' attribute has no file associated with it. 
# File objects can be assigned to FileField attributes,  but shouldn't get

# committed until the model it's attached to is saved.



>>> from django.core.files.uploadedfile import SimpleUploadedFile

>>> obj1.normal = SimpleUploadedFile('assignment.txt', 'content')

>>> dirs, files = temp_storage.listdir('tests')

>>> dirs
@@ -93,16 +91,17 @@ ValueError: The 'normal' attribute has no file associated with it. 
>>> obj2 = Storage()

>>> obj2.normal.save('django_test.txt', ContentFile('more content'))

>>> obj2.normal

<FieldFile: tests/django_test_.txt>

<FieldFile: tests/django_test_1.txt>

>>> obj2.normal.size

12



# Push the objects into the cache to make sure they pickle properly



>>> from django.core.cache import cache

>>> 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,7 +109,17 @@ ValueError: The 'normal' attribute has no file associated with it. 
>>> obj2.delete()

>>> obj2.normal.save('django_test.txt', ContentFile('more content'))

>>> obj2.normal

<FieldFile: tests/django_test_.txt>

<FieldFile: tests/django_test_1.txt>



# Multiple files with the same name get _N appended to them.



>>> objs = [Storage() for i in range(3)] 

>>> for o in objs:  

...     o.normal.save('multiple_files.txt', ContentFile('Same Content')) 

>>> [o.normal for o in objs]

[<FieldFile: tests/multiple_files.txt>, <FieldFile: tests/multiple_files_1.txt>, <FieldFile: tests/multiple_files_2.txt>]

>>> for o in objs: 

...     o.delete() 



# Default values allow an object to access a single file.
@@ -139,5 +148,7 @@ ValueError: The 'normal' attribute has no file associated with it. 
>>> obj2.normal.delete()

>>> obj3.default.delete()

>>> obj4.random.delete()



>>> import shutil

>>> shutil.rmtree(temp_storage_location)

"""}

tests/regressiontests/file_storage/tests.py

+4 −4
Original line number Diff line number Diff line
@@ -126,9 +126,9 @@ class FileSaveRaceConditionTest(TestCase): 
        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):
@@ -167,7 +167,7 @@ class FileStoragePathParsing(TestCase): 


        self.failIf(os.path.exists(os.path.join(self.storage_dir, 'dotted_.path')))

        self.assert_(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/test')))

        self.assert_(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/test_')))

        self.assert_(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/test_1')))



    def test_first_character_dot(self):

        """
@@ -183,7 +183,7 @@ class FileStoragePathParsing(TestCase): 
        if sys.version_info < (2, 6):

            self.assert_(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/_.test')))

        else:

            self.assert_(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/.test_')))

            self.assert_(os.path.exists(os.path.join(self.storage_dir, 'dotted.path/.test_1')))



if Image is not None:

    class DimensionClosingBug(TestCase):