Transitions the rest of the code to use FileResponse instead of a basic response, fixes up tests which assumed .content exists

This commit is contained in:
stumpylog 2026-04-24 11:06:09 -07:00 committed by Trenton H
parent 661bd064e7
commit 44d1a23b63
6 changed files with 39 additions and 34 deletions

View File

@ -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())

View File

@ -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(

View File

@ -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(

View File

@ -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)

View File

@ -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

View File

@ -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