Skip to content

Commit

Permalink
[PP-1947] convert attachments to s3 links (#2209)
Browse files Browse the repository at this point in the history
  • Loading branch information
dbernstein authored Dec 17, 2024
1 parent 20b157f commit ff63d23
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import csv
import tempfile
import uuid
import zipfile
from datetime import datetime
from pathlib import Path
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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}")
Expand Down Expand Up @@ -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()
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -290,15 +306,32 @@ 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"]
)
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"]
)
15 changes: 15 additions & 0 deletions tests/manager/service/storage/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit ff63d23

Please sign in to comment.