[Django] #35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used

35 views
Skip to first unread message

Django

unread,
Mar 22, 2024, 11:56:38 AM3/22/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
------------------------------------------------+------------------------
Reporter: bcail | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
[https://code.djangoproject.com/ticket/28144 Ticket #28144] added the
option of using custom flags on a storage object to allow overwriting
files in storage. However, this doesn't seem to work for temporary
uploaded files, since the
[https://github.com/django/django/blob/main/django/core/files/storage/filesystem.py#L100
alternate path] is taken in the _save method.

Here is an example test that fails for me - it loops forever:
{{{
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
index 420314573d..d404300708 100644
--- a/tests/file_storage/tests.py
+++ b/tests/file_storage/tests.py
@@ -648,6 +648,34 @@ class OverwritingStorageTests(FileStorageTests):
finally:
self.storage.delete(name)

+ def test_save_overwrite_behavior_temp_file(self):
+ """Saving to same file name twice overwrites the first file."""
+ name = "test.file"
+ self.assertFalse(self.storage.exists(name))
+ content_1 = b"content one"
+ content_2 = b"second content"
+ f_1 = TemporaryUploadedFile('tmp1', 'text/plain', 11, 'utf8')
+ f_1.write(content_1)
+ f_1.seek(0)
+ f_2 = TemporaryUploadedFile('tmp2', 'text/plain', 14, 'utf8')
+ f_2.write(content_2)
+ f_2.seek(0)
+ stored_name_1 = self.storage.save(name, f_1)
+ try:
+ self.assertEqual(stored_name_1, name)
+ self.assertTrue(self.storage.exists(name))
+ self.assertTrue(os.path.exists(os.path.join(self.temp_dir,
name)))
+ with self.storage.open(name) as fp:
+ self.assertEqual(fp.read(), content_1)
+ stored_name_2 = self.storage.save(name, f_2)
+ self.assertEqual(stored_name_2, name)
+ self.assertTrue(self.storage.exists(name))
+ self.assertTrue(os.path.exists(os.path.join(self.temp_dir,
name)))
+ with self.storage.open(name) as fp:
+ self.assertEqual(fp.read(), content_2)
+ finally:
+ self.storage.delete(name)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 24, 2024, 1:43:07 PM3/24/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: nobody
Type: Bug | Status: new
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by alexialg05):

can i be assigned on this issue?
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:1>

Django

unread,
Mar 25, 2024, 8:26:05 AM3/25/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: nobody
Type: Bug | Status: new
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by bcail):

@alexialg05, yes, you can assign yourself to this issue, but we should
probably see if others confirm this as an issue. If it is confirmed as an
issue, there may be design questions to figure out.
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:2>

Django

unread,
Mar 25, 2024, 10:20:22 AM3/25/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* cc: Tim Graham (added)
* stage: Unreviewed => Accepted

Comment:

Hello bcail, thank you for your report!

This definitely seems like a valid issue, `file_move_safe` (from
`django.core.files.move`) requires the `allow_overwrite` flag to be passed
to perform overwrites. I don't have clarity on the possible fix, I agree
that it requires a design decision. A forum post seeking advice would be
the best next step, I think.
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:3>

Django

unread,
Mar 25, 2024, 10:31:46 AM3/25/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by bcail):

Thanks, @Natalia. I opened a new
[https://forum.djangoproject.com/t/issue-35326-overwritingstoragetests-
design-discussion/29462 forum post].
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:4>

Django

unread,
Mar 26, 2024, 11:37:25 AM3/26/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by bcail):

I opened a [https://github.com/django/django/pull/18020 draft PR] with a
possible direction we could go.
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:5>

Django

unread,
Mar 27, 2024, 11:00:03 AM3/27/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* has_patch: 0 => 1
* owner: nobody => bcail
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:6>

Django

unread,
Apr 24, 2024, 4:24:54 AM4/24/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:7>

Django

unread,
Apr 30, 2024, 11:12:29 AM4/30/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by bcail):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:8>

Django

unread,
Apr 30, 2024, 1:07:29 PM4/30/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:9>

Django

unread,
Apr 30, 2024, 2:00:32 PM4/30/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by bcail):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:10>

Django

unread,
May 4, 2024, 12:19:10 PM5/4/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:11>

Django

unread,
May 9, 2024, 2:17:02 PM5/9/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
--------------------------------------+------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by bcail):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:12>

Django

unread,
May 16, 2024, 5:31:49 AM5/16/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: assigned
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:13>

Django

unread,
May 21, 2024, 1:28:24 AM5/21/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: closed
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"0b33a3abc2ca7d68a24f6d0772bc2b9fa603744e" 0b33a3a]:
{{{#!CommitTicketReference repository=""
revision="0b33a3abc2ca7d68a24f6d0772bc2b9fa603744e"
Fixed #35326 -- Added allow_overwrite parameter to FileSystemStorage.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:14>

Django

unread,
May 21, 2024, 5:23:59 AM5/21/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: closed
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"480ccf905560e2bc63be8cfd24355c557d404677" 480ccf90]:
{{{#!CommitTicketReference repository=""
revision="480ccf905560e2bc63be8cfd24355c557d404677"
Refs #35326 -- Made cosmetic edits to 5.1 release notes.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:15>

Django

unread,
Jul 24, 2024, 8:55:20 AM7/24/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: closed
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"8d6a20b656ff3fa18e36954668a44a831c2f6ddd" 8d6a20b]:
{{{#!CommitTicketReference repository=""
revision="8d6a20b656ff3fa18e36954668a44a831c2f6ddd"
Fixed #35604, Refs #35326 -- Made FileSystemStorage.exists() behaviour
independent from allow_overwrite.

Partially reverts 0b33a3abc2ca7d68a24f6d0772bc2b9fa603744e.

Storage.exists(name) was documented to "return False if
the name is available for a new file." but return True if
the file exists. This is ambiguous in the overwrite file
case. It will now always return whether the file exists.

Thank you to Natalia Bidart and Josh Schneier for the
review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:16>

Django

unread,
Jul 24, 2024, 8:59:45 AM7/24/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: closed
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"e42defb63b765b589fb8ef54e2d6773999d41939" e42defb]:
{{{#!CommitTicketReference repository=""
revision="e42defb63b765b589fb8ef54e2d6773999d41939"
[5.1.x] Fixed #35604, Refs #35326 -- Made FileSystemStorage.exists()
behaviour independent from allow_overwrite.

Partially reverts 0b33a3abc2ca7d68a24f6d0772bc2b9fa603744e.

Storage.exists(name) was documented to "return False if
the name is available for a new file." but return True if
the file exists. This is ambiguous in the overwrite file
case. It will now always return whether the file exists.

Thank you to Natalia Bidart and Josh Schneier for the
review.

Backport of 8d6a20b656ff3fa18e36954668a44a831c2f6ddd from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:17>

Django

unread,
Aug 28, 2024, 10:44:15 AM8/28/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: closed
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by nessita <124304+nessita@…>):

In [changeset:"47f18a722624527cc72eef44cfc9d1e07ea4b4e0" 47f18a72]:
{{{#!CommitTicketReference repository=""
revision="47f18a722624527cc72eef44cfc9d1e07ea4b4e0"
Refs #35326 -- Adjusted deprecation warning stacklevel in
FileSystemStorage.OS_OPEN_FLAGS.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:18>

Django

unread,
Aug 28, 2024, 10:48:13 AM8/28/24
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: closed
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):

In [changeset:"8f5d2c374a150fca063443292cdf2618026bda42" 8f5d2c37]:
{{{#!CommitTicketReference repository=""
revision="8f5d2c374a150fca063443292cdf2618026bda42"
[5.1.x] Refs #35326 -- Adjusted deprecation warning stacklevel in
FileSystemStorage.OS_OPEN_FLAGS.

Backport of 47f18a722624527cc72eef44cfc9d1e07ea4b4e0 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:19>

Django

unread,
Jan 15, 2025, 4:28:47 PMJan 15
to django-...@googlegroups.com
#35326: OverwritingStorageTests fail if a TemporaryUploadedFile is used
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: Bug | Status: closed
Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"17ae61a5d4ec75ac5e40363cc76b04c191b50d3d" 17ae61a5]:
{{{#!CommitTicketReference repository=""
revision="17ae61a5d4ec75ac5e40363cc76b04c191b50d3d"
Refs #35326 -- Removed FileSystemStorage.OS_OPEN_FLAGS per deprecation
timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35326#comment:20>
Reply all
Reply to author
Forward
0 new messages