Skip to content

Commit

Permalink
[s3] fix file reopening (#1321)
Browse files Browse the repository at this point in the history
  • Loading branch information
jschneier committed Oct 9, 2023
1 parent c15d7b0 commit 5744f08
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
22 changes: 19 additions & 3 deletions storages/backends/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,32 @@ def __init__(self, name, mode, storage, buffer_size=None):
self._storage.get_object_parameters(self.name)
)
self.obj.load(**params)
self._is_dirty = False
self._raw_bytes_written = 0
self._closed = False
self._file = None
self._multipart = None
self._parts = None
# 5 MB is the minimum part size (if there is more than one part).
# Amazon allows up to 10,000 parts. The default supports uploads
# up to roughly 50 GB. Increase the part size to accommodate
# for files larger than this.
self.buffer_size = buffer_size or setting("AWS_S3_FILE_BUFFER_SIZE", 5242880)
self._reset_file_properties()

def _reset_file_properties(self):
self._multipart = None
self._raw_bytes_written = 0
self._write_counter = 0
self._is_dirty = False

def open(self, mode=None):
if self._file is not None and not self.closed:
self.seek(0) # Mirror Django's behavior
elif mode and mode != self._mode:
raise ValueError("Cannot reopen file with a new mode.")

# Accessing the file will functionally re-open it
self.file # noqa: B018

return self

@property
def size(self):
Expand Down Expand Up @@ -264,6 +278,8 @@ def close(self):
if self._file is not None:
self._file.close()
self._file = None

self._reset_file_properties()
self._closed = True


Expand Down
17 changes: 17 additions & 0 deletions tests/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,8 @@ def test_save(self):


class S3FileTests(TestCase):
# Remove the override_settings after Python3.7 is dropped
@override_settings(AWS_S3_OBJECT_PARAMETERS={"ContentType": "text/html"})
def setUp(self) -> None:
self.storage = s3.S3Storage()
self.storage._connections.connection = mock.MagicMock()
Expand Down Expand Up @@ -1015,6 +1017,21 @@ def test_closed(self):
f.file
self.assertFalse(f.closed)

def test_reopening(self):
f = s3.S3File("test", "wb", self.storage)

with f.open() as fp:
fp.write(b"xyz")

with f.open() as fp:
fp.write(b"xyz")

# Properties are reset
self.assertEqual(f._write_counter, 0)
self.assertEqual(f._raw_bytes_written, 0)
self.assertFalse(f._is_dirty)
self.assertIsNone(f._multipart)


@mock_s3
class S3StorageTestsWithMoto(TestCase):
Expand Down

0 comments on commit 5744f08

Please sign in to comment.