From 44d1a23b63a8fb89cebca8aeec435892b2762496 Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Fri, 24 Apr 2026 11:06:09 -0700 Subject: [PATCH] Transitions the rest of the code to use FileResponse instead of a basic response, fixes up tests which assumed .content exists --- src/documents/tests/test_api_bulk_download.py | 21 ++++++-------- .../tests/test_api_document_versions.py | 7 +++-- src/documents/tests/test_api_documents.py | 28 +++++++++---------- src/documents/tests/test_views.py | 3 +- src/documents/tests/utils.py | 8 ++++++ src/documents/views.py | 6 ++-- 6 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/documents/tests/test_api_bulk_download.py b/src/documents/tests/test_api_bulk_download.py index 115e33804..eae03a3ed 100644 --- a/src/documents/tests/test_api_bulk_download.py +++ b/src/documents/tests/test_api_bulk_download.py @@ -15,6 +15,7 @@ from documents.models import Document from documents.models import DocumentType from documents.tests.utils import DirectoriesMixin from documents.tests.utils import SampleDirMixin +from documents.tests.utils import read_streaming_response class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): @@ -56,12 +57,6 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): shutil.copy(self.SAMPLE_DIR / "simple.jpg", self.doc3.source_path) shutil.copy(self.SAMPLE_DIR / "test_with_bom.pdf", self.doc3.archive_path) - @staticmethod - def _zip_content(response) -> io.BytesIO: - content = b"".join(response.streaming_content) - response.close() - return io.BytesIO(content) - def test_download_originals(self) -> None: response = self.client.post( self.ENDPOINT, @@ -74,7 +69,7 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response["Content-Type"], "application/zip") - with zipfile.ZipFile(self._zip_content(response)) as zipf: + with zipfile.ZipFile(io.BytesIO(read_streaming_response(response))) as zipf: self.assertEqual(len(zipf.filelist), 2) self.assertIn("2021-01-01 document A.pdf", zipf.namelist()) self.assertIn("2020-03-21 document B.jpg", zipf.namelist()) @@ -95,7 +90,7 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response["Content-Type"], "application/zip") - with zipfile.ZipFile(self._zip_content(response)) as zipf: + with zipfile.ZipFile(io.BytesIO(read_streaming_response(response))) as zipf: self.assertEqual(len(zipf.filelist), 2) self.assertIn("2021-01-01 document A.pdf", zipf.namelist()) self.assertIn("2020-03-21 document B.pdf", zipf.namelist()) @@ -116,7 +111,7 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response["Content-Type"], "application/zip") - with zipfile.ZipFile(self._zip_content(response)) as zipf: + with zipfile.ZipFile(io.BytesIO(read_streaming_response(response))) as zipf: self.assertEqual(len(zipf.filelist), 3) self.assertIn("originals/2021-01-01 document A.pdf", zipf.namelist()) self.assertIn("archive/2020-03-21 document B.pdf", zipf.namelist()) @@ -150,7 +145,7 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response["Content-Type"], "application/zip") - with zipfile.ZipFile(self._zip_content(response)) as zipf: + with zipfile.ZipFile(io.BytesIO(read_streaming_response(response))) as zipf: self.assertEqual(len(zipf.filelist), 2) self.assertIn("2021-01-01 document A.pdf", zipf.namelist()) @@ -210,7 +205,7 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response["Content-Type"], "application/zip") - with zipfile.ZipFile(self._zip_content(response)) as zipf: + with zipfile.ZipFile(io.BytesIO(read_streaming_response(response))) as zipf: self.assertEqual(len(zipf.filelist), 2) self.assertIn("a space name/Title 2 - Doc 3.jpg", zipf.namelist()) self.assertIn("test/This is Doc 2.pdf", zipf.namelist()) @@ -256,7 +251,7 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response["Content-Type"], "application/zip") - with zipfile.ZipFile(self._zip_content(response)) as zipf: + with zipfile.ZipFile(io.BytesIO(read_streaming_response(response))) as zipf: self.assertEqual(len(zipf.filelist), 2) self.assertIn("somewhere/This is Doc 2.pdf", zipf.namelist()) self.assertIn("somewhere/Title 2 - Doc 3.pdf", zipf.namelist()) @@ -305,7 +300,7 @@ class TestBulkDownload(DirectoriesMixin, SampleDirMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response["Content-Type"], "application/zip") - with zipfile.ZipFile(self._zip_content(response)) as zipf: + with zipfile.ZipFile(io.BytesIO(read_streaming_response(response))) as zipf: self.assertEqual(len(zipf.filelist), 3) self.assertIn("originals/bill/This is Doc 2.pdf", zipf.namelist()) self.assertIn("archive/statement/Title 2 - Doc 3.pdf", zipf.namelist()) diff --git a/src/documents/tests/test_api_document_versions.py b/src/documents/tests/test_api_document_versions.py index bde13354a..848c6ec21 100644 --- a/src/documents/tests/test_api_document_versions.py +++ b/src/documents/tests/test_api_document_versions.py @@ -18,6 +18,7 @@ from documents.filters import EffectiveContentFilter from documents.filters import TitleContentFilter from documents.models import Document from documents.tests.utils import DirectoriesMixin +from documents.tests.utils import read_streaming_response if TYPE_CHECKING: from pathlib import Path @@ -449,19 +450,19 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): f"/api/documents/{root.id}/download/?version={version.id}", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(resp.content, b"version") + self.assertEqual(read_streaming_response(resp), b"version") resp = self.client.get( f"/api/documents/{root.id}/preview/?version={version.id}", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(resp.content, b"version") + self.assertEqual(read_streaming_response(resp), b"version") resp = self.client.get( f"/api/documents/{root.id}/thumb/?version={version.id}", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(resp.content, b"thumb") + self.assertEqual(read_streaming_response(resp), b"thumb") def test_metadata_version_param_uses_version(self) -> None: root = Document.objects.create( diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py index 24165c087..844dc2af5 100644 --- a/src/documents/tests/test_api_documents.py +++ b/src/documents/tests/test_api_documents.py @@ -49,6 +49,7 @@ from documents.models import WorkflowTrigger from documents.signals.handlers import run_workflows from documents.tests.utils import ConsumeTaskMixin from documents.tests.utils import DirectoriesMixin +from documents.tests.utils import read_streaming_response class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): @@ -323,19 +324,16 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): f.write(content_thumbnail) response = self.client.get(f"/api/documents/{doc.pk}/download/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content) + self.assertEqual(read_streaming_response(response), content) response = self.client.get(f"/api/documents/{doc.pk}/preview/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content) + self.assertEqual(read_streaming_response(response), content) response = self.client.get(f"/api/documents/{doc.pk}/thumb/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content_thumbnail) + self.assertEqual(read_streaming_response(response), content_thumbnail) def test_document_actions_with_perms(self) -> None: """ @@ -386,12 +384,15 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): response = self.client.get(f"/api/documents/{doc.pk}/download/") self.assertEqual(response.status_code, status.HTTP_200_OK) + response.close() response = self.client.get(f"/api/documents/{doc.pk}/preview/") self.assertEqual(response.status_code, status.HTTP_200_OK) + response.close() response = self.client.get(f"/api/documents/{doc.pk}/thumb/") self.assertEqual(response.status_code, status.HTTP_200_OK) + response.close() @override_settings(FILENAME_FORMAT="") def test_download_with_archive(self) -> None: @@ -412,28 +413,24 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): f.write(content_archive) response = self.client.get(f"/api/documents/{doc.pk}/download/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content_archive) + self.assertEqual(read_streaming_response(response), content_archive) response = self.client.get( f"/api/documents/{doc.pk}/download/?original=true", ) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content) + self.assertEqual(read_streaming_response(response), content) response = self.client.get(f"/api/documents/{doc.pk}/preview/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content_archive) + self.assertEqual(read_streaming_response(response), content_archive) response = self.client.get( f"/api/documents/{doc.pk}/preview/?original=true", ) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content) + self.assertEqual(read_streaming_response(response), content) @override_settings(FILENAME_FORMAT="") def test_download_follow_formatting(self) -> None: @@ -456,18 +453,21 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): # Without follow_formatting, should use public filename response = self.client.get(f"/api/documents/{doc.pk}/download/") self.assertIn("none.pdf", response["Content-Disposition"]) + response.close() # With follow_formatting, should use actual filename on disk response = self.client.get( f"/api/documents/{doc.pk}/download/?follow_formatting=true", ) self.assertIn("archived.pdf", response["Content-Disposition"]) + response.close() # With follow_formatting and original, should use source filename response = self.client.get( f"/api/documents/{doc.pk}/download/?original=true&follow_formatting=true", ) self.assertIn("my_document.pdf", response["Content-Disposition"]) + response.close() def test_document_actions_not_existing_file(self) -> None: doc = Document.objects.create( diff --git a/src/documents/tests/test_views.py b/src/documents/tests/test_views.py index 314636045..9b1724a16 100644 --- a/src/documents/tests/test_views.py +++ b/src/documents/tests/test_views.py @@ -27,6 +27,7 @@ from documents.models import StoragePath from documents.models import Tag from documents.signals.handlers import update_llm_suggestions_cache from documents.tests.utils import DirectoriesMixin +from documents.tests.utils import read_streaming_response from paperless.models import ApplicationConfiguration @@ -157,7 +158,7 @@ class TestViews(DirectoriesMixin, TestCase): # Valid response = self.client.get(f"/share/{sl1.slug}") self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.content, content) + self.assertEqual(read_streaming_response(response), content) # Invalid response = self.client.get("/share/123notaslug", follow=True) diff --git a/src/documents/tests/utils.py b/src/documents/tests/utils.py index 530f588e8..38e7c927a 100644 --- a/src/documents/tests/utils.py +++ b/src/documents/tests/utils.py @@ -17,6 +17,7 @@ import pytest from django.apps import apps from django.db import connection from django.db.migrations.executor import MigrationExecutor +from django.http import StreamingHttpResponse from django.test import TransactionTestCase from django.test import override_settings @@ -150,6 +151,13 @@ def util_call_with_backoff( return succeeded, result +def read_streaming_response(response: StreamingHttpResponse) -> bytes: + """Consume a StreamingHttpResponse/FileResponse and close it.""" + content = b"".join(response.streaming_content) + response.close() + return content + + class DirectoriesMixin: """ Creates and overrides settings for all folders and paths, then ensures diff --git a/src/documents/views.py b/src/documents/views.py index 34eebf66f..4aeda1011 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1436,7 +1436,7 @@ class DocumentViewSet( file_doc = self._get_effective_file_doc(request_doc, root_doc, request) handle = file_doc.thumbnail_file - return HttpResponse(handle, content_type="image/webp") + return FileResponse(handle, content_type="image/webp") except FileNotFoundError: raise Http404 @@ -4281,7 +4281,7 @@ def serve_file( use_archive: bool, disposition: str, follow_formatting: bool = False, -) -> HttpResponse: +) -> FileResponse: if use_archive: if TYPE_CHECKING: assert doc.archive_filename @@ -4304,7 +4304,7 @@ def serve_file( if mime_type in {"application/csv", "text/csv"} and disposition == "inline": mime_type = "text/plain" - response = HttpResponse(file_handle, content_type=mime_type) + response = FileResponse(file_handle, content_type=mime_type) # Firefox is not able to handle unicode characters in filename field # RFC 5987 addresses this issue # see https://datatracker.ietf.org/doc/html/rfc5987#section-4.2