Code

Ticket #2983: 2983.2.diff

File 2983.2.diff, 19.6 KB (added by SmileyChris, 5 years ago)
Line 
1### Eclipse Workspace Patch 1.0
2#P Django trunk
3Index: tests/regressiontests/file_uploads/tests.py
4===================================================================
5--- tests/regressiontests/file_uploads/tests.py (revision 11327)
6+++ tests/regressiontests/file_uploads/tests.py (working copy)
7@@ -12,7 +12,7 @@
8 from django.utils.hashcompat import sha_constructor
9 from django.http.multipartparser import MultiPartParser
10 
11-from models import FileModel, temp_storage, UPLOAD_TO
12+from models import FileModel, FileModelDeleteReplaced, temp_storage, UPLOAD_TO
13 import uploadhandler
14 
15 UNICODE_FILENAME = u'test-0123456789_中文_Orléans.jpg'
16@@ -302,3 +302,216 @@
17             'CONTENT_TYPE':     'multipart/form-data; boundary=_foo',
18             'CONTENT_LENGTH':   '1'
19         }, StringIO('x'), [], 'utf-8')
20+
21+class DeleteReplacedTests(TestCase):
22+    def setUp(self):
23+        if not os.path.isdir(temp_storage.location):
24+            os.makedirs(temp_storage.location)
25+        if os.path.isdir(UPLOAD_TO):
26+            os.chmod(UPLOAD_TO, 0700)
27+            shutil.rmtree(UPLOAD_TO)
28+        self.file_a = SimpleUploadedFile('alpha.txt', 'A')
29+        self.file_b = SimpleUploadedFile('beta.txt', 'B')
30+        self.file_g = SimpleUploadedFile('gamma.txt', 'G')
31+
32+    def tearDown(self):
33+        os.chmod(temp_storage.location, 0700)
34+        shutil.rmtree(temp_storage.location)
35+
36+    def test_instance_track_replaced(self):
37+        """
38+        Setting a new file into a instance's ``FileField`` attribute keeps
39+        track of the old file in the new file's ``_replaced`` list.
40+        """
41+        obj = FileModel()
42+        obj.testfile = self.file_a
43+        fieldfile_a = obj.testfile
44+        self.assertEqual(obj.testfile._replaced, [])
45+        # After a save, nothing changes.
46+        obj.save()
47+        self.assertEqual(obj.testfile._replaced, [])
48+        # Set to B, B replaces A
49+        obj.testfile = self.file_b
50+        fieldfile_b = obj.testfile
51+        self.assertEqual(obj.testfile._replaced, [fieldfile_a])
52+        # Set to G, G replaces B (which in turn replaces A)
53+        obj.testfile = self.file_g
54+        fieldfile_g = obj.testfile
55+        self.assertEqual(obj.testfile._replaced, [fieldfile_b])
56+        self.assertEqual(obj.testfile._replaced[0]._replaced, [fieldfile_a])
57+
58+    def test_default_on_delete(self):
59+        """
60+        Deleting a FieldFile which has a default FileField doesn't delete
61+        replaced files.
62+        """
63+        obj = FileModel.objects.create(testfile=self.file_a)
64+        obj.testfile = self.file_b
65+        obj.testfile.delete()
66+        self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt'])
67+
68+    def test_delete_replaced_on_delete(self):
69+        """
70+        Deleting a FieldFile which its FileField set ``delete_replaced=True``
71+        deletes replaced files (without any "safe" tests).
72+        """
73+        obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a)
74+        obj.testfile = self.file_b
75+        obj.testfile.delete()
76+        self.assertEqual(os.listdir(UPLOAD_TO), [])
77+        # Even if another instance has a reference to the file it will still be
78+        # deleted.
79+        obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a)
80+        obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile)
81+        obj.testfile = self.file_b
82+        obj.testfile.delete()
83+        self.assertEqual(os.listdir(UPLOAD_TO), [])
84+
85+    def test_default_on_safe_delete(self):
86+        """
87+        Calling ``safe_delete`` on a FieldFile which has a default FileField
88+        doesn't delete replaced files.
89+        """
90+        obj = FileModel.objects.create(testfile=self.file_a)
91+        obj.testfile = self.file_b
92+        obj.testfile.safe_delete()
93+        self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt'])
94+        # If another instance has a reference to the file it won't be deleted.
95+        obj = FileModel.objects.create(testfile=self.file_b)
96+        obj.testfile = self.file_g
97+        obj2 = FileModel.objects.create(testfile=obj.testfile)
98+        obj.testfile.safe_delete()
99+        files = os.listdir(UPLOAD_TO)
100+        files.sort()
101+        self.assertEqual(files, ['alpha.txt', 'beta.txt', 'gamma.txt'])
102+
103+    def test_delete_replaced_on_safe_delete(self):
104+        """
105+        Deleting a FieldFile which its FileField set ``delete_replaced=True``
106+        "safely" deletes replaced files.
107+        """
108+        obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a)
109+        obj.testfile = self.file_b
110+        obj.testfile.safe_delete()
111+        self.assertEqual(os.listdir(UPLOAD_TO), [])
112+        # If another instance has a reference to the file it won't be deleted.
113+        obj = FileModelDeleteReplaced.objects.create(testfile=self.file_b)
114+        obj.testfile = self.file_g
115+        obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile)
116+        obj.testfile.safe_delete()
117+        self.assertEqual(os.listdir(UPLOAD_TO), ['gamma.txt'])
118+
119+    def test_default_on_save(self):
120+        """
121+        Saving a FieldFile which has a default FileField doesn't delete
122+        replaced files.
123+        """
124+        obj = FileModel.objects.create(testfile=self.file_a)
125+        obj.testfile.save(name=self.file_b.name, content=self.file_b,
126+                          save=False)
127+        files = os.listdir(UPLOAD_TO)
128+        files.sort()
129+        self.assertEqual(files, ['alpha.txt', 'beta.txt'])
130+
131+    def test_delete_replaced_on_save(self):
132+        """
133+        Saving a FieldFile which its FileField set ``delete_replaced=True``
134+        deletes replaced files.
135+       
136+        The ``safe_delete_replaced`` attribute defaults to ``True``, which
137+        causes related files to be "safely" deleted. Setting to ``False``
138+        causes related files to be deleted without any "safe" tests.
139+        """
140+        obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a)
141+        obj.testfile.save(name=self.file_b.name, content=self.file_b,
142+                          save=False)
143+        self.assertEqual(os.listdir(UPLOAD_TO), ['beta.txt'])
144+        # If another instance has a reference to the file it won't be deleted.
145+        obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a)
146+        obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile)
147+        obj.testfile.save(name=self.file_g.name, content=self.file_g,
148+                          save=False)
149+        files = os.listdir(UPLOAD_TO)
150+        files.sort()
151+        self.assertEqual(files, ['alpha.txt', 'beta.txt', 'gamma.txt'])
152+
153+    def test_delete_replaced(self):
154+        """
155+        Calling the ``delete_replaced`` method of a ``FieldFile`` recursively
156+        deletes replaced files (regardless of the `FileField``'s
157+        ``delete_replaced`` attribute).
158+       
159+        The ``safe_delete`` argument defaults to ``True``, which causes related
160+        files to be "safely" deleted. Setting to ``False`` causes related files
161+        to be deleted without any "safe" tests.
162+        """
163+        obj = FileModel.objects.create(testfile=self.file_a)
164+        obj.testfile = self.file_b
165+        obj.save()
166+        obj.testfile = self.file_g
167+        obj.save()
168+        files = os.listdir(UPLOAD_TO)
169+        files.sort()
170+        self.assertEqual(files, ['alpha.txt', 'beta.txt', 'gamma.txt'])
171+        obj.testfile.delete_replaced()
172+        self.assertEqual(os.listdir(UPLOAD_TO), ['gamma.txt'])
173+
174+    def test_replace_avoids_loop(self):
175+        """
176+        Avoid an infinite loop when A replaces B which replaced A
177+        """
178+        obj = FileModel.objects.create(testfile=self.file_a)
179+        fieldfile_a = obj.testfile
180+        obj.testfile = self.file_b
181+        obj.save()
182+        obj.testfile = fieldfile_a
183+        obj.testfile.delete_replaced()
184+        self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt'])
185+
186+    def test_instance_delete(self):
187+        """
188+        Deleting an instance deletes replaced files. For backwards
189+        compatibility, this is regardless of the `FileField``'s
190+        ``delete_replaced`` attribute. Files are only deleted if no other
191+        instance of the same model type references that file.
192+        """
193+        obj = FileModel.objects.create(testfile=self.file_a)
194+        obj.delete()
195+        self.assertEqual(os.listdir(UPLOAD_TO), [])
196+
197+        obj = FileModel.objects.create(testfile=self.file_a)
198+        obj2 = FileModel.objects.create(testfile=obj.testfile)
199+        self.assertEqual(os.listdir(UPLOAD_TO), ['alpha.txt'])
200+
201+    def test_default_instance_replace_file(self):
202+        """
203+        Saving a FieldFile which has a default FileField doesn't delete
204+        replaced files when the instance is saved.
205+        """
206+        obj = FileModel.objects.create(testfile=self.file_a)
207+        obj.testfile = self.file_b
208+        obj.save()
209+        files = os.listdir(UPLOAD_TO)
210+        files.sort()
211+        self.assertEqual(files, ['alpha.txt', 'beta.txt'])
212+
213+    def test_delete_replaced_instance_replace_file(self):
214+        """
215+        If the model's FileField sets ``delete_replaced=True``, replacing an
216+        instance's file with another file will cause the old file to be deleted
217+        when the instance is saved.
218+       
219+        Files are only deleted if no other instance of the same model type
220+        references that file.
221+        """
222+        obj = FileModelDeleteReplaced.objects.create(testfile=self.file_a)
223+        obj.testfile = self.file_b
224+        obj.save()
225+        self.assertEqual(os.listdir(UPLOAD_TO), ['beta.txt'])
226+
227+        obj2 = FileModelDeleteReplaced.objects.create(testfile=obj.testfile)
228+        obj.testfile = self.file_g
229+        obj.save()
230+        files = os.listdir(UPLOAD_TO)
231+        files.sort()
232+        self.assertEqual(files, ['beta.txt', 'gamma.txt'])
233Index: django/db/models/fields/files.py
234===================================================================
235--- django/db/models/fields/files.py    (revision 11327)
236+++ django/db/models/fields/files.py    (working copy)
237@@ -22,6 +22,8 @@
238         self.field = field
239         self.storage = field.storage
240         self._committed = True
241+        self._replaced = []
242+        self.delete_replaced_files = self.field.delete_replaced
243 
244     def __eq__(self, other):
245         # Older code may be expecting FileField values to be simple strings.
246@@ -86,10 +88,19 @@
247     # to further manipulate the underlying file, as well as update the
248     # associated model instance.
249 
250-    def save(self, name, content, save=True):
251+    def save(self, name, content, save=True, safe_delete_replaced=True):
252+        if self.delete_replaced_files:
253+            self.delete_replaced(safe_delete=safe_delete_replaced)
254+            if self._committed:
255+                # Delete this file as well (since we're saving a new one).
256+                if safe_delete_replaced:
257+                    self.safe_delete(save=False)
258+                else:
259+                    self.delete(save=False)
260+
261         name = self.field.generate_filename(self.instance, name)
262         self.name = self.storage.save(name, content)
263-        setattr(self.instance, self.field.name, self.name)
264+        setattr(self.instance, self.field.name, self)
265 
266         # Update the filesize cache
267         self._size = len(content)
268@@ -100,7 +111,17 @@
269             self.instance.save()
270     save.alters_data = True
271 
272-    def delete(self, save=True):
273+    def delete(self, save=True, _delete_replaced_files=None):
274+        """
275+        Deletes the file from the backend.
276+       
277+        If ``save`` is ``True`` (default), the file's instance will be saved
278+        after the file is deleted.
279+        """
280+        if ((_delete_replaced_files is not None and _delete_replaced_files) or
281+            (_delete_replaced_files is None and self.delete_replaced_files)):
282+            self.delete_replaced(safe_delete=False)
283+
284         # Only close the file if it's already open, which we know by the
285         # presence of self._file
286         if hasattr(self, '_file'):
287@@ -110,7 +131,7 @@
288         self.storage.delete(self.name)
289 
290         self.name = None
291-        setattr(self.instance, self.field.name, self.name)
292+        setattr(self.instance, self.field.name, self)
293 
294         # Delete the filesize cache
295         if hasattr(self, '_size'):
296@@ -121,6 +142,49 @@
297             self.instance.save()
298     delete.alters_data = True
299 
300+    def safe_delete(self, save=True, queryset=None,
301+                    _delete_replaced_files=None):
302+        """
303+        Deletes the file from the backend if no objects in the queryset
304+        reference the file and it's not the default value for future objects.
305+
306+        Otherwise, the file is simply closed so it doesn't tie up resources.
307+       
308+        If ``save`` is ``True`` (default), the file's instance will be saved
309+        if the file is deleted.
310+
311+        Under most circumstances, ``queryset`` does not need to be passed -
312+        it will be calculated based on the current instance.
313+        """
314+        if ((_delete_replaced_files is not None and _delete_replaced_files) or
315+            (_delete_replaced_files is None and self.delete_replaced_files)):
316+            self.delete_replaced(safe_delete=True)
317+
318+        if queryset is None:
319+            queryset = self.instance._default_manager.all()
320+            if self.instance.pk:
321+                queryset = queryset.exclude(pk=self.instance.pk)
322+        queryset = queryset.filter(**{self.field.name: self.name})
323+
324+        if self.name != self.field.default and not queryset:
325+            self.delete(save=save, _delete_replaced_files=False)
326+        else:
327+            self.close()
328+    safe_delete.alters_data = True
329+
330+    def delete_replaced(self, safe_delete=True, _seen=None):
331+        seen = _seen or []
332+        seen.append(self.name)
333+        for file in self._replaced:
334+            if file._committed and file.name not in seen:
335+                file.delete_replaced(safe_delete=safe_delete, _seen=seen)
336+                if safe_delete:
337+                    file.safe_delete(save=False, _delete_replaced_files=False)
338+                else:
339+                    file.delete(save=False, _delete_replaced_files=False)
340+        self._replaced = []
341+    delete_replaced.alters_data = True
342+
343     def _get_closed(self):
344         file = getattr(self, '_file', None)
345         return file is None or file.closed
346@@ -136,7 +200,9 @@
347         # it's attached to in order to work properly, but the only necessary
348         # data to be pickled is the file's name itself. Everything else will
349         # be restored later, by FileDescriptor below.
350-        return {'name': self.name, 'closed': False, '_committed': True, '_file': None}
351+        return {'name': self.name, 'closed': False, '_committed': True,
352+                '_file': None,
353+                'delete_replaced_files': self.delete_replaced_files}
354 
355 class FileDescriptor(object):
356     """
357@@ -194,19 +260,41 @@
358             file_copy._committed = False
359             instance.__dict__[self.field.name] = file_copy
360 
361-        # Finally, because of the (some would say boneheaded) way pickle works,
362-        # the underlying FieldFile might not actually itself have an associated
363-        # file. So we need to reset the details of the FieldFile in those cases.
364+        # Because of the (some would say boneheaded) way pickle works, the
365+        # underlying FieldFile might not actually itself have an associated
366+        # file. So we need to reset the details of the FieldFile in those
367+        # cases.
368         elif isinstance(file, FieldFile) and not hasattr(file, 'field'):
369             file.instance = instance
370             file.field = self.field
371             file.storage = self.field.storage
372 
373+        # Finally, the file set may have been a FieldFile from another
374+        # instance, so copy it if the instance doesn't match.
375+        elif isinstance(file, FieldFile) and file.instance != instance:
376+            file_copy = self.field.attr_class(instance, self.field, file.name)
377+            file_copy.file = file
378+            file_copy._committed = file._committed
379+            instance.__dict__[self.field.name] = file_copy
380+
381         # That was fun, wasn't it?
382         return instance.__dict__[self.field.name]
383 
384     def __set__(self, instance, value):
385+        if self.field.name in instance.__dict__:
386+            previous_file = getattr(instance, self.field.name)
387+        else:
388+            previous_file = None
389         instance.__dict__[self.field.name] = value
390+        if previous_file:
391+            # Rather than just using value, we get the file from the instance,
392+            # so that the __get__ logic of the file descriptor is processed.
393+            # This ensures we will be dealing with a FileField (or subclass of
394+            # FileField) instance.
395+            file = getattr(instance, self.field.name)
396+            if previous_file is not file:
397+                # Remember that the previous file was replaced.
398+                file._replaced.append(previous_file)
399 
400 class FileField(Field):
401     # The class to wrap instance attributes in. Accessing the file object off
402@@ -216,7 +304,8 @@
403     # The descriptor to use for accessing the attribute off of the class.
404     descriptor_class = FileDescriptor
405 
406-    def __init__(self, verbose_name=None, name=None, upload_to='', storage=None, **kwargs):
407+    def __init__(self, verbose_name=None, name=None, upload_to='',
408+                 storage=None, delete_replaced=False, **kwargs):
409         for arg in ('primary_key', 'unique'):
410             if arg in kwargs:
411                 raise TypeError("'%s' is not a valid argument for %s." % (arg, self.__class__))
412@@ -225,6 +314,7 @@
413         self.upload_to = upload_to
414         if callable(upload_to):
415             self.generate_filename = upload_to
416+        self.delete_replaced = delete_replaced
417 
418         kwargs['max_length'] = kwargs.get('max_length', 100)
419         super(FileField, self).__init__(verbose_name, name, **kwargs)
420@@ -258,16 +348,14 @@
421         signals.post_delete.connect(self.delete_file, sender=cls)
422 
423     def delete_file(self, instance, sender, **kwargs):
424+        """
425+        Signal receiver which deletes an attached file from the backend when
426+        the model is deleted.
427+        """
428         file = getattr(instance, self.attname)
429-        # If no other object of this type references the file,
430-        # and it's not the default value for future objects,
431-        # delete it from the backend.
432-        if file and file.name != self.default and \
433-            not sender._default_manager.filter(**{self.name: file.name}):
434-                file.delete(save=False)
435-        elif file:
436-            # Otherwise, just close the file, so it doesn't tie up resources.
437-            file.close()
438+        if file:
439+            file.safe_delete(save=False,
440+                             queryset=sender._default_manager.all())
441 
442     def get_directory_name(self):
443         return os.path.normpath(force_unicode(datetime.datetime.now().strftime(smart_str(self.upload_to))))
444Index: tests/regressiontests/file_uploads/models.py
445===================================================================
446--- tests/regressiontests/file_uploads/models.py        (revision 11327)
447+++ tests/regressiontests/file_uploads/models.py        (working copy)
448@@ -4,7 +4,12 @@
449 from django.core.files.storage import FileSystemStorage
450 
451 temp_storage = FileSystemStorage(tempfile.mkdtemp())
452-UPLOAD_TO = os.path.join(temp_storage.location, 'test_upload')
453+UPLOAD_TO_NAME = 'test_upload'
454+UPLOAD_TO = os.path.join(temp_storage.location, UPLOAD_TO_NAME)
455 
456 class FileModel(models.Model):
457-    testfile = models.FileField(storage=temp_storage, upload_to='test_upload')
458+    testfile = models.FileField(storage=temp_storage, upload_to=UPLOAD_TO_NAME)
459+
460+class FileModelDeleteReplaced(models.Model):
461+    testfile = models.FileField(storage=temp_storage, upload_to=UPLOAD_TO_NAME,
462+                                delete_replaced=True)