From 5366aa96fef0ce55fc425cf273c7debe74d99305 Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Tue, 23 Feb 2010 22:39:22 +0000 Subject: [PATCH] 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 --- django/core/files/storage.py | 10 +++++---- docs/howto/custom-file-storage.txt | 4 ++-- tests/modeltests/files/models.py | 23 +++++++++++++++------ tests/regressiontests/file_storage/tests.py | 8 +++---- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/django/core/files/storage.py b/django/core/files/storage.py index 96c0b54623..d312681c28 100644 --- a/django/core/files/storage.py +++ b/django/core/files/storage.py @@ -1,6 +1,7 @@ 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): diff --git a/docs/howto/custom-file-storage.txt b/docs/howto/custom-file-storage.txt index cfb3226289..5005feaa80 100644 --- a/docs/howto/custom-file-storage.txt +++ b/docs/howto/custom-file-storage.txt @@ -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. diff --git a/tests/modeltests/files/models.py b/tests/modeltests/files/models.py index beea97e808..271fd8de9e 100644 --- a/tests/modeltests/files/models.py +++ b/tests/modeltests/files/models.py @@ -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 - + >>> 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 - + # 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 - + + +# 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] +[, , ] +>>> 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) """} diff --git a/tests/regressiontests/file_storage/tests.py b/tests/regressiontests/file_storage/tests.py index 81ea48950c..b1e31d1811 100644 --- a/tests/regressiontests/file_storage/tests.py +++ b/tests/regressiontests/file_storage/tests.py @@ -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):