diff --git a/CHANGELOG.md b/CHANGELOG.md index c452c25..faee2a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +- Improve performance of streamed file downloads changed in v0.8 ([#44](https://github.com/torchbox/wagtail-bynder/pull/44/) @alxbridge + ## [0.8] - 2025-11-12 ### Fixed -- Improved handling for unexpected server error responses when downloading Bynder assets ([#43](hhttps://github.com/torchbox/wagtail-bynder/pull/43/)) @alxbridge +- Improved handling for unexpected server error responses when downloading Bynder assets ([#43](https://github.com/torchbox/wagtail-bynder/pull/43/)) @alxbridge ## [0.7] - 2025-01-15 diff --git a/src/wagtail_bynder/utils.py b/src/wagtail_bynder/utils.py index d86ed3b..a3bce5a 100644 --- a/src/wagtail_bynder/utils.py +++ b/src/wagtail_bynder/utils.py @@ -36,9 +36,8 @@ def download_file( file = BytesIO() # Stream the file to memory. We use iter_content() instead of the default iterator for requests.Response, # as the latter uses iter_lines() which isn't suitable for streaming binary data. - for chunk in response.iter_content(): - if not chunk: - continue + # Get data in largish 8KB chunks, for more performant streaming while staying within CPU cache limits + for chunk in response.iter_content(chunk_size=8192): file.write(chunk) if file.tell() > max_filesize: file.truncate(0) @@ -47,6 +46,11 @@ def download_file( ) size = file.tell() + # Catch empty case where iter_content wouldn't have iterated + if size == 0: + raise BynderAssetDownloadError( + f"Downloaded file '{name}' from Bynder is empty." + ) file.seek(0) content_type, charset = mimetypes.guess_type(name) diff --git a/tests/test_utils.py b/tests/test_utils.py index 264a812..49f320e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -24,6 +24,22 @@ def test_download_file_raises_error_on_non_200_status(self): self.assertIn("file.jpg", str(cm.exception)) self.assertIn("Server error downloading", str(cm.exception)) + def test_download_file_raises_error_on_empty_response(self): + """Test that download_file raises BynderAssetDownloadError for successful-but-empty responses""" + mock_response = mock.Mock() + mock_response.status_code = 200 + # Mock iter_content to return no chunks (empty file) + mock_response.iter_content = mock.Mock(return_value=[]) + + with ( + mock.patch("wagtail_bynder.utils.requests.get", return_value=mock_response), + self.assertRaises(BynderAssetDownloadError) as cm, + ): + download_file("https://example.com/empty.jpg", 5242880, "TEST_SETTING") + + self.assertIn("empty.jpg", str(cm.exception)) + self.assertIn("empty", str(cm.exception).lower()) + def test_download_file_succeeds_on_200(self): """Test that download_file works correctly with 200 status""" mock_response = mock.Mock()