Account for the streaming nature

src/documents/tests/test_api_bulk_download.py

@@ -80,7 +80,7 @@ class TestBulkDownload(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response["Content-Type"], "application/zip")
 
-        with zipfile.ZipFile(io.BytesIO(response.content)) as zipf:
+        with zipfile.ZipFile(io.BytesIO(response.getvalue())) 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())
@@ -101,7 +101,7 @@ class TestBulkDownload(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response["Content-Type"], "application/zip")
 
-        with zipfile.ZipFile(io.BytesIO(response.content)) as zipf:
+        with zipfile.ZipFile(io.BytesIO(response.getvalue())) 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())
@@ -122,7 +122,7 @@ class TestBulkDownload(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response["Content-Type"], "application/zip")
 
-        with zipfile.ZipFile(io.BytesIO(response.content)) as zipf:
+        with zipfile.ZipFile(io.BytesIO(response.getvalue())) 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())
@@ -156,7 +156,7 @@ class TestBulkDownload(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response["Content-Type"], "application/zip")
 
-        with zipfile.ZipFile(io.BytesIO(response.content)) as zipf:
+        with zipfile.ZipFile(io.BytesIO(response.getvalue())) as zipf:
             self.assertEqual(len(zipf.filelist), 2)
 
             self.assertIn("2021-01-01 document A.pdf", zipf.namelist())
@@ -215,7 +215,7 @@ class TestBulkDownload(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response["Content-Type"], "application/zip")
 
-        with zipfile.ZipFile(io.BytesIO(response.content)) as zipf:
+        with zipfile.ZipFile(io.BytesIO(response.getvalue())) 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())
@@ -261,7 +261,7 @@ class TestBulkDownload(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response["Content-Type"], "application/zip")
 
-        with zipfile.ZipFile(io.BytesIO(response.content)) as zipf:
+        with zipfile.ZipFile(io.BytesIO(response.getvalue())) 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())
@@ -310,7 +310,7 @@ class TestBulkDownload(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response["Content-Type"], "application/zip")
 
-        with zipfile.ZipFile(io.BytesIO(response.content)) as zipf:
+        with zipfile.ZipFile(io.BytesIO(response.getvalue())) 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())

src/documents/views.py

@@ -33,6 +33,7 @@ from django.db.models import When
 from django.db.models.functions import Length
 from django.db.models.functions import Lower
 from django.db.models.manager import Manager
+from django.http import FileResponse
 from django.http import Http404
 from django.http import HttpResponse
 from django.http import HttpResponseBadRequest
@@ -1595,10 +1596,10 @@ class BulkDownloadView(GenericAPIView):
                 return HttpResponseForbidden("Insufficient permissions")
 
         settings.SCRATCH_DIR.mkdir(parents=True, exist_ok=True)
-        temp = tempfile.NamedTemporaryFile(  # noqa: SIM115
-            dir=settings.SCRATCH_DIR,
-            suffix="-compressed-archive",
-            delete=False,
+        temp_dir = Path(
+            tempfile.mkdtemp(
+                dir=settings.SCRATCH_DIR,
+            ),
         )
 
         if content == "both":
@@ -1608,20 +1609,14 @@ class BulkDownloadView(GenericAPIView):
         else:
             strategy_class = ArchiveOnlyStrategy
 
-        with zipfile.ZipFile(temp.name, "w", compression) as zipf:
+        zip_file = temp_dir / "documents.zip"
+
+        with zipfile.ZipFile(zip_file, "w", compression) as zipf:
             strategy = strategy_class(zipf, follow_filename_format)
             for document in documents:
                 strategy.add_document(document)
 
-        # TODO(stumpylog): Investigate using FileResponse here
-        with open(temp.name, "rb") as f:
-            response = HttpResponse(f, content_type="application/zip")
-            response["Content-Disposition"] = '{}; filename="{}"'.format(
-                "attachment",
-                "documents.zip",
-            )
-
-            return response
+        return FileResponse(zip_file.open("rb"), as_attachment=True)
 
 
 class StoragePathViewSet(ModelViewSet, PermissionsAwareDocumentCountMixin):