Index: django/db/models/fields/files.py =================================================================== --- django/db/models/fields/files.py (revision 13350) +++ django/db/models/fields/files.py (working copy) @@ -23,6 +23,8 @@ self.field = field self.storage = field.storage self._committed = True + self._replaced = [] + self.delete_replaced_files = self.field.delete_replaced def __eq__(self, other): # Older code may be expecting FileField values to be simple strings. @@ -88,9 +90,15 @@ # associated model instance. def save(self, name, content, save=True): + if self.delete_replaced_files: + self.delete_replaced() + if self._committed: + # Delete this file as well (since we're saving a new one). + self.delete_if_orphan(save=False) + name = self.field.generate_filename(self.instance, name) self.name = self.storage.save(name, content) - setattr(self.instance, self.field.name, self.name) + setattr(self.instance, self.field.name, self) # Update the filesize cache self._size = len(content) @@ -101,7 +109,22 @@ self.instance.save() save.alters_data = True - def delete(self, save=True): + def delete(self, save=True, delete_replaced_files=None): + """ + Deletes the file from the backend. + + If ``save`` is ``True`` (default), the file's instance will be saved + after the file is deleted. + + ``delete_replaced_files`` determines whether to also delete replaced + files which are orphans. If set to ``None``, the field's default + setting applies. + """ + if delete_replaced_files is None: + delete_replaced_files = self.delete_replaced_files + if delete_replaced_files: + self.delete_replaced() + # Only close the file if it's already open, which we know by the # presence of self._file if hasattr(self, '_file'): @@ -111,7 +134,7 @@ self.storage.delete(self.name) self.name = None - setattr(self.instance, self.field.name, self.name) + setattr(self.instance, self.field.name, self) # Delete the filesize cache if hasattr(self, '_size'): @@ -122,6 +145,55 @@ self.instance.save() delete.alters_data = True + def delete_if_orphan(self, save=True, queryset=None, + delete_replaced_files=None): + """ + Deletes the file from the backend if no objects in the queryset + reference the file and it's not the default value for future objects. + + Otherwise, the file is simply closed so it doesn't tie up resources. + + If ``save`` is ``True`` (default), the file's instance will be saved + if the file is deleted. + + Under most circumstances, ``queryset`` does not need to be passed - + it will be calculated based on the current instance. + + ``delete_replaced_files`` determines whether to also delete replaced + files which are orphans. If set to ``None``, the field's default + setting applies. + """ + if delete_replaced_files is None: + delete_replaced_files = self.delete_replaced_files + if delete_replaced_files: + self.delete_replaced() + + if queryset is None: + queryset = self.instance._default_manager.all() + if self.instance.pk: + queryset = queryset.exclude(pk=self.instance.pk) + queryset = queryset.filter(**{self.field.name: self.name}) + + if self.name != self.field.default and not queryset: + self.delete(save=save, delete_replaced_files=False) + else: + self.close() + delete_if_orphan.alters_data = True + + def delete_replaced(self, only_orphans=True, _seen=None): + seen = _seen or [] + seen.append(self.name) + for file in self._replaced: + if file._committed and file.name not in seen: + file.delete_replaced(only_orphans=only_orphans, _seen=seen) + if only_orphans: + file.delete_if_orphan(save=False, + delete_replaced_files=False) + else: + file.delete(save=False, delete_replaced_files=False) + self._replaced = [] + delete_replaced.alters_data = True + def _get_closed(self): file = getattr(self, '_file', None) return file is None or file.closed @@ -137,7 +209,9 @@ # it's attached to in order to work properly, but the only necessary # data to be pickled is the file's name itself. Everything else will # be restored later, by FileDescriptor below. - return {'name': self.name, 'closed': False, '_committed': True, '_file': None} + return {'name': self.name, 'closed': False, '_committed': True, + '_file': None, + 'delete_replaced_files': self.delete_replaced_files} class FileDescriptor(object): """ @@ -195,19 +269,41 @@ file_copy._committed = False instance.__dict__[self.field.name] = file_copy - # Finally, because of the (some would say boneheaded) way pickle works, - # the underlying FieldFile might not actually itself have an associated - # file. So we need to reset the details of the FieldFile in those cases. + # Because of the (some would say boneheaded) way pickle works, the + # underlying FieldFile might not actually itself have an associated + # file. So we need to reset the details of the FieldFile in those + # cases. elif isinstance(file, FieldFile) and not hasattr(file, 'field'): file.instance = instance file.field = self.field file.storage = self.field.storage + # Finally, the file set may have been a FieldFile from another + # instance, so copy it if the instance doesn't match. + elif isinstance(file, FieldFile) and file.instance != instance: + file_copy = self.field.attr_class(instance, self.field, file.name) + file_copy.file = file + file_copy._committed = file._committed + instance.__dict__[self.field.name] = file_copy + # That was fun, wasn't it? return instance.__dict__[self.field.name] def __set__(self, instance, value): + if self.field.name in instance.__dict__: + previous_file = getattr(instance, self.field.name) + else: + previous_file = None instance.__dict__[self.field.name] = value + if previous_file: + # Rather than just using value, we get the file from the instance, + # so that the __get__ logic of the file descriptor is processed. + # This ensures we will be dealing with a FileField (or subclass of + # FileField) instance. + file = getattr(instance, self.field.name) + if previous_file is not file: + # Remember that the previous file was replaced. + file._replaced.append(previous_file) class FileField(Field): # The class to wrap instance attributes in. Accessing the file object off @@ -219,7 +315,8 @@ description = ugettext_lazy("File path") - def __init__(self, verbose_name=None, name=None, upload_to='', storage=None, **kwargs): + def __init__(self, verbose_name=None, name=None, upload_to='', + storage=None, delete_replaced=False, **kwargs): for arg in ('primary_key', 'unique'): if arg in kwargs: raise TypeError("'%s' is not a valid argument for %s." % (arg, self.__class__)) @@ -228,6 +325,7 @@ self.upload_to = upload_to if callable(upload_to): self.generate_filename = upload_to + self.delete_replaced = delete_replaced kwargs['max_length'] = kwargs.get('max_length', 100) super(FileField, self).__init__(verbose_name, name, **kwargs) @@ -261,16 +359,14 @@ signals.post_delete.connect(self.delete_file, sender=cls) def delete_file(self, instance, sender, **kwargs): + """ + Signal receiver which deletes an attached file from the backend when + the model is deleted. + """ file = getattr(instance, self.attname) - # If no other object of this type references the file, - # and it's not the default value for future objects, - # delete it from the backend. - if file and file.name != self.default and \ - not sender._default_manager.filter(**{self.name: file.name}): - file.delete(save=False) - elif file: - # Otherwise, just close the file, so it doesn't tie up resources. - file.close() + if file: + file.delete_if_orphan(save=False, + queryset=sender._default_manager.all()) def get_directory_name(self): return os.path.normpath(force_unicode(datetime.datetime.now().strftime(smart_str(self.upload_to)))) Index: tests/regressiontests/file_uploads/models.py =================================================================== --- tests/regressiontests/file_uploads/models.py (revision 13350) +++ tests/regressiontests/file_uploads/models.py (working copy) @@ -4,7 +4,12 @@ from django.core.files.storage import FileSystemStorage temp_storage = FileSystemStorage(tempfile.mkdtemp()) -UPLOAD_TO = os.path.join(temp_storage.location, 'test_upload') +UPLOAD_TO_NAME = 'test_upload' +UPLOAD_TO = os.path.join(temp_storage.location, UPLOAD_TO_NAME) class FileModel(models.Model): - testfile = models.FileField(storage=temp_storage, upload_to='test_upload') + testfile = models.FileField(storage=temp_storage, upload_to=UPLOAD_TO_NAME) + +class FileModelDeleteReplaced(models.Model): + testfile = models.FileField(storage=temp_storage, upload_to=UPLOAD_TO_NAME, + delete_replaced=True) Index: tests/regressiontests/file_uploads/tests.py =================================================================== --- tests/regressiontests/file_uploads/tests.py (revision 13350) +++ tests/regressiontests/file_uploads/tests.py (working copy) @@ -12,7 +12,7 @@ from django.utils.hashcompat import sha_constructor from django.http.multipartparser import MultiPartParser -from models import FileModel, temp_storage, UPLOAD_TO +from models import FileModel, FileModelDeleteReplaced, temp_storage, UPLOAD_TO import uploadhandler UNICODE_FILENAME = u'test-0123456789_中文_Orléans.jpg' @@ -302,3 +302,249 @@ 'CONTENT_TYPE': 'multipart/form-data; boundary=_foo', 'CONTENT_LENGTH': '1' }, StringIO('x'), [], 'utf-8') + +class DeleteReplacedTests(TestCase): + def setUp(self): + if not os.path.isdir(temp_storage.location): + os.makedirs(temp_storage.location) + if os.path.isdir(UPLOAD_TO): + os.chmod(UPLOAD_TO, 0700) + shutil.rmtree(UPLOAD_TO) + self.file_a = SimpleUploadedFile('alpha.txt', 'A') + self.file_b = SimpleUploadedFile('beta.txt', 'B') + self.file_g = SimpleUploadedFile('gamma.txt', 'G') + + def tearDown(self): + os.chmod(temp_storage.location, 0700) + shutil.rmtree(temp_storage.location) + + def test_instance_track_replaced(self): + """ + Setting a new file into a instance's ``FileField`` attribute keeps + track of the old file in the new file's ``_replaced`` list. + """ + obj = FileModel() + obj.testfile = self.file_a + fieldfile_a = obj.testfile + self.assertEqual(obj.testfile._replaced, []) + # After a save, nothing changes. + obj.save() + self.assertEqual(obj.testfile._replaced, []) + # Set to B, B replaces A + obj.testfile = self.file_b + fieldfile_b = obj.testfile + self.assertEqual(obj.testfile._replaced, [fieldfile_a]) + # Set to G, G replaces B (which in turn replaces A) + obj.testfile = self.file_g + fieldfile_g = obj.testfile + self.assertEqual(obj.testfile._replaced, [fieldfile_b]) + self.assertEqual(obj.testfile._replaced[0]._replaced, [fieldfile_a]) + + def test_default_on_delete(self): + """ + Deleting a FieldFile with a standard FileField doesn't delete replaced + files by default. To delete replaced files explicitly, you can set + ``delete_replaced_files=True``. + """ + obj = FileModel.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.testfile.delete() + self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt']) + # Explicitly delete replaced files. + obj.testfile.delete(delete_replaced_files=True) + self.assertEqual(os.listdir(UPLOAD_TO), []) + + def test_delete_replaced_on_delete(self): + """ + Deleting a FieldFile which its FileField set ``delete_replaced=True`` + deletes replaced files which are orphans. + """ + obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.testfile.delete() + self.assertEqual(os.listdir(UPLOAD_TO), []) + # If another instance has a reference to the file it won't be deleted. + obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a) + obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile) + obj.testfile = self.file_b + obj.testfile.delete() + self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt']) + + def test_default_on_delete_if_orphan(self): + """ + Calling ``delete_if_orphan`` on a FieldFile with a standard FileField + doesn't delete replaced files by default. To delete replaced files + explicitly, you can set ``delete_replaced_files=True``. + """ + obj = FileModel.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.testfile.delete_if_orphan() + expected = ['alpha.txt'] + self.assertEqual(os.listdir(UPLOAD_TO), expected) + # If another instance has a reference to the file it won't be deleted. + obj = FileModel.objects.create(testfile=self.file_b) + obj.testfile = self.file_g + obj2 = FileModel.objects.create(testfile=obj.testfile) + obj.testfile.delete_if_orphan() + files = os.listdir(UPLOAD_TO) + files.sort() + expected.extend(['beta.txt', 'gamma.txt']) + self.assertEqual(files, expected) + # Explicitly delete replaced files. + obj.testfile.delete(delete_replaced_files=True) + expected.remove('beta.txt') + self.assertEqual(os.listdir(UPLOAD_TO), expected) + + def test_delete_replaced_on_delete_if_orphan(self): + """ + Deleting a FieldFile which its FileField set ``delete_replaced=True`` + deletes replaced files which are orphans. + """ + obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.testfile.delete() + self.assertEqual(os.listdir(UPLOAD_TO), []) + # If another instance has a reference to the file it won't be deleted. + obj = FileModelDeleteReplaced.objects.create(testfile=self.file_b) + obj.testfile = self.file_g + obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile) + obj.testfile.delete_if_orphan() + self.assertEqual(os.listdir(UPLOAD_TO), ['gamma.txt']) + + def test_default_on_save(self): + """ + Saving a FieldFile with a standard FileField doesn't delete replaced + files. + """ + obj = FileModel.objects.create(testfile=self.file_a) + obj.testfile.save(name=self.file_b.name, content=self.file_b, + save=False) + files = os.listdir(UPLOAD_TO) + files.sort() + self.assertEqual(files, ['alpha.txt', 'beta.txt']) + + def test_delete_replaced_on_save(self): + """ + Saving a FieldFile which its FileField set ``delete_replaced=True`` + deletes replaced files which are orphans. + """ + obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a) + obj.testfile.save(name=self.file_b.name, content=self.file_b, + save=False) + expected = ['beta.txt'] + self.assertEqual(os.listdir(UPLOAD_TO), expected) + # If another instance has a reference to the file it won't be deleted. + obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a) + obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile) + obj.testfile.save(name=self.file_g.name, content=self.file_g, + save=False) + files = os.listdir(UPLOAD_TO) + files.sort() + expected.extend(['alpha.txt', 'gamma.txt']) + expected.sort() + self.assertEqual(files, expected) + + def test_delete_replaced(self): + """ + Explicitly calling the ``delete_replaced`` method of a ``FieldFile`` + recursively deletes replaced files which are orphans. + + This happens regardless of the `FileField``'s ``delete_replaced`` + attribute. + + To delete all replaced files without considering if they are orphans, + set ``only_orphans=False``. + """ + obj = FileModel.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.save() + obj2 = FileModel.objects.create(testfile=obj.testfile) + obj.testfile = self.file_g + obj.save() + files = os.listdir(UPLOAD_TO) + files.sort() + self.assertEqual(files, ['alpha.txt', 'beta.txt', 'gamma.txt']) + # Now delete_replaced. + obj.testfile.delete_replaced() + files = os.listdir(UPLOAD_TO) + files.sort() + self.assertEqual(files, ['beta.txt', 'gamma.txt']) + + def test_delete_replaced_all(self): + """ + Calling the ``delete_replaced`` method of a ``FieldFile`` with + ``only_orphans=False`` recursively deletes replaced files without + considering if they are orphans. + """ + obj = FileModel.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.save() + obj2 = FileModel.objects.create(testfile=obj.testfile) + obj.testfile = self.file_g + obj.save() + files = os.listdir(UPLOAD_TO) + files.sort() + self.assertEqual(files, ['alpha.txt', 'beta.txt', 'gamma.txt']) + # Now delete_replaced. + obj.testfile.delete_replaced(only_orphans=False) + self.assertEqual(os.listdir(UPLOAD_TO), ['gamma.txt']) + + def test_replace_avoids_loop(self): + """ + Avoid an infinite loop when A replaces B which replaced A + """ + obj = FileModel.objects.create(testfile=self.file_a) + fieldfile_a = obj.testfile + obj.testfile = self.file_b + obj.save() + obj.testfile = fieldfile_a + obj.testfile.delete_replaced() + self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt']) + + def test_instance_delete(self): + """ + Deleting an instance deletes replaced files. For backwards + compatibility, this is regardless of the `FileField``'s + ``delete_replaced`` attribute. Files are only deleted if no other + instance of the same model type references that file. + """ + obj = FileModel.objects.create(testfile=self.file_a) + obj.delete() + self.assertEqual(os.listdir(UPLOAD_TO), []) + + obj = FileModel.objects.create(testfile=self.file_a) + obj2 = FileModel.objects.create(testfile=obj.testfile) + self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt']) + + def test_default_instance_replace_file(self): + """ + Saving a model with a standard FileField doesn't delete replaced files + when the instance is saved. + """ + obj = FileModel.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.save() + files = os.listdir(UPLOAD_TO) + files.sort() + self.assertEqual(files, ['alpha.txt', 'beta.txt']) + + def test_delete_replaced_instance_replace_file(self): + """ + If the model's FileField sets ``delete_replaced=True``, replacing an + instance's file with another file will cause the old file to be deleted + when the instance is saved. + + Files are only deleted if no other instance of the same model type + references that file. + """ + obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a) + obj.testfile = self.file_b + obj.save() + self.assertEqual(os.listdir(UPLOAD_TO), ['beta.txt']) + + obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile) + obj.testfile = self.file_g + obj.save() + files = os.listdir(UPLOAD_TO) + files.sort() + self.assertEqual(files, ['beta.txt', 'gamma.txt']) Index: docs/ref/models/fields.txt =================================================================== --- docs/ref/models/fields.txt (revision 13350) +++ docs/ref/models/fields.txt (working copy) @@ -465,7 +465,7 @@ ``FileField`` ------------- -.. class:: FileField(upload_to=None, [max_length=100, **options]) +.. class:: FileField(upload_to=None, [max_length=100, storage=None, delete_replaced=False, **options]) A file-upload field. @@ -511,7 +511,7 @@ when determining the final destination path. ====================== =============================================== -Also has one optional argument: +Also has two optional arguments: .. attribute:: FileField.storage @@ -520,6 +520,15 @@ Optional. A storage object, which handles the storage and retrieval of your files. See :ref:`topics-files` for details on how to provide this object. +.. attribute:: FileField.delete_replaced + + .. versionadded:: 1.3 + + Delete replaced files (if they are `orphaned`__) when the model instance is + saved. + + .. __: `deletion of orphaned files`_ + The admin represents this field as an ```` (a file-upload widget). @@ -611,6 +620,29 @@ The optional ``save`` argument controls whether or not the instance is saved after the file has been deleted. Defaults to ``True``. +Deletion of Orphaned Files +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When a model is deleted, the underlying file is deleted if it is considered an +orphan. + +A file's orphan status is decided by checking the field all other instances for +this model for a reference to the same file. + +To remove this default behavior, remove the signal:: + + from django.db.signals import post_delete + post_delete.disconnect(SomeModel._meta.get_field('the_file_field').delete_file) + +.. versionadded:: 1.3 + Deleting replaced files is now possible. + +When a file field is changed on a model instance, the replaced file it is *not* +deleted by default. + +To automatically delete replaced files that are orphaned when the model instance +is saved, set ``delete_replaced=True`` on the ``FileField``. + ``FilePathField`` -----------------