From ff63d23d9df7c3d18aa523e6ea1787b46111edab Mon Sep 17 00:00:00 2001 From: dbernstein Date: Mon, 16 Dec 2024 16:30:15 -0800 Subject: [PATCH] [PP-1947] convert attachments to s3 links (#2209) --- .../generate_inventory_and_hold_reports.py | 31 ++++++++---- ...est_generate_inventory_and_hold_reports.py | 47 ++++++++++++++++--- tests/manager/service/storage/test_s3.py | 15 ++++++ 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py b/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py index d53731e86..7453f561d 100644 --- a/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py +++ b/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py @@ -2,6 +2,7 @@ import csv import tempfile +import uuid import zipfile from datetime import datetime from pathlib import Path @@ -20,6 +21,7 @@ from palace.manager.service.integration_registry.license_providers import ( LicenseProvidersRegistry, ) +from palace.manager.service.storage.s3 import S3Service from palace.manager.sqlalchemy.model.integration import ( IntegrationConfiguration, IntegrationLibraryConfiguration, @@ -71,14 +73,14 @@ def __init__( email_address: str, send_email: SendEmailCallable, registry: LicenseProvidersRegistry, - delete_attachments: bool = True, + s3_service: S3Service, ): super().__init__(session_maker) self.library_id = library_id self.email_address = email_address - self.delete_attachments = delete_attachments self.send_email = send_email self.registry = registry + self.s3_service = s3_service def run(self) -> None: with self.transaction() as session: @@ -113,9 +115,7 @@ def run(self) -> None: "integration_ids": tuple(integration_ids), } - with tempfile.NamedTemporaryFile( - delete=self.delete_attachments - ) as report_zip: + with tempfile.NamedTemporaryFile() as report_zip: zip_path = Path(report_zip.name) with ( @@ -148,13 +148,25 @@ def run(self) -> None: arcname=f"palace-inventory-report-for-library-{file_name_modifier}.csv", ) + uid = uuid.uuid4() + key = ( + f"inventory_and_holds/{library.short_name}/" + f"inventory-and-holds-for-library-{file_name_modifier}-{uid}.zip" + ) + self.s3_service.store_stream( + key, + report_zip, # type: ignore[arg-type] + content_type="application/zip", + ) + + s3_file_link = self.s3_service.generate_url(key) self.send_email( subject=f"Inventory and Holds Reports {current_time}", receivers=[self.email_address], - text="", - attachments={ - f"palace-inventory-and-holds-reports-for-{file_name_modifier}.zip": zip_path - }, + text=( + f"Download Report here -> {s3_file_link} \n\n" + f"This report will be available for download for 30 days." + ), ) self.log.debug(f"Zip file written to {zip_path}") @@ -331,4 +343,5 @@ def generate_inventory_and_hold_reports( email_address=email_address, send_email=task.services.email.send_email, registry=task.services.integration_registry.license_providers(), + s3_service=task.services.storage.public(), ).run() diff --git a/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py b/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py index c5496d646..1f5908ecf 100644 --- a/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py +++ b/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py @@ -3,8 +3,8 @@ import os import zipfile from datetime import timedelta -from typing import IO -from unittest.mock import create_autospec +from typing import IO, BinaryIO +from unittest.mock import MagicMock, create_autospec from pytest import LogCaptureFixture from sqlalchemy.orm import sessionmaker @@ -40,12 +40,16 @@ def test_job_run( send_email_mock = create_autospec( services_fixture.services.email.container.send_email ) + + mock_s3 = MagicMock() + GenerateInventoryAndHoldsReportsJob( mock_session_maker, library_id=1, email_address=email, send_email=send_email_mock, registry=services_fixture.services.integration_registry.license_providers(), + s3_service=mock_s3, ).run() assert ( f"Cannot generate inventory and holds report for library (id=1): library not found." @@ -187,19 +191,31 @@ def test_job_run( library.id, email_address=email, send_email=send_email_mock, - delete_attachments=False, registry=services_fixture.services.integration_registry.license_providers(), + s3_service=mock_s3, ) + reports_zip = "test_zip" + + def store_stream_mock( + key: str, + stream: BinaryIO, + content_type: str | None = None, + ): + + with open(reports_zip, "wb") as file: + file.write(stream.read()) + + mock_s3.store_stream = store_stream_mock + job.run() + + mock_s3.generate_url.assert_called_once() send_email_mock.assert_called_once() kwargs = send_email_mock.call_args.kwargs assert kwargs["receivers"] == [email] assert "Inventory and Holds Reports" in kwargs["subject"] - attachments: dict = kwargs["attachments"] - - assert len(attachments) == 1 - reports_zip = list(attachments.values())[0] + assert "This report will be available for download for 30 days." in kwargs["text"] try: with zipfile.ZipFile(reports_zip, mode="r") as archive: entry_list = archive.namelist() @@ -290,11 +306,20 @@ def test_generate_inventory_and_hold_reports_task( services_fixture: ServicesFixture, celery_fixture: CeleryFixture, ): + + mock_s3_service = MagicMock() + mock_s3_service.generate_url.return_value = "http://test" + services_fixture.services.storage.public.override(mock_s3_service) + library = db.library(short_name="test_library") # there must be at least one opds collection associated with the library for this to work create_test_opds_collection("c1", "d1", db, library) generate_inventory_and_hold_reports.delay(library.id, "test@email").wait() services_fixture.email_fixture.mock_emailer.send.assert_called_once() + + mock_s3_service.store_stream.assert_called_once() + mock_s3_service.generate_url.assert_called_once() + assert ( "Inventory and Holds Reports" in services_fixture.email_fixture.mock_emailer.send.call_args.kwargs["subject"] @@ -302,3 +327,11 @@ def test_generate_inventory_and_hold_reports_task( assert services_fixture.email_fixture.mock_emailer.send.call_args.kwargs[ "receivers" ] == ["test@email"] + assert ( + "Download Report here -> http://test" + in services_fixture.email_fixture.mock_emailer.send.call_args.kwargs["text"] + ) + assert ( + "This report will be available for download for 30 days." + in services_fixture.email_fixture.mock_emailer.send.call_args.kwargs["text"] + ) diff --git a/tests/manager/service/storage/test_s3.py b/tests/manager/service/storage/test_s3.py index cce3aee37..f8265266f 100644 --- a/tests/manager/service/storage/test_s3.py +++ b/tests/manager/service/storage/test_s3.py @@ -225,6 +225,21 @@ def test_multipart_upload_exception(self, s3_service_fixture: S3ServiceFixture): with pytest.raises(RuntimeError): upload.upload_part(b"foo") + def _configuration(self, prefix, expiration): + return { + "Rules": [ + { + "Expiration": { + "Days": expiration, + }, + "ID": f"expiration_on_{prefix}", + "Prefix": prefix, + "Filter": {"Prefix": prefix}, + "Status": "Enabled", + } + ] + } + @pytest.mark.minio class TestS3ServiceIntegration: