diff --git a/object_storage_api/schemas/image.py b/object_storage_api/schemas/image.py index 5f25e06..7b5c4ca 100644 --- a/object_storage_api/schemas/image.py +++ b/object_storage_api/schemas/image.py @@ -38,4 +38,5 @@ class ImageMetadataSchema(CreatedModifiedSchemaMixin, ImagePostMetadataSchema): class ImageSchema(ImageMetadataSchema): """Schema model for an image get request response.""" - url: HttpUrl = Field(description="Presigned get URL to get the image file") + view_url: HttpUrl = Field(description="Presigned get URL to view the image file") + download_url: HttpUrl = Field(description="Presigned get URL to download the image file") diff --git a/object_storage_api/services/image.py b/object_storage_api/services/image.py index 4c92914..da95074 100644 --- a/object_storage_api/services/image.py +++ b/object_storage_api/services/image.py @@ -82,14 +82,14 @@ def create(self, image_metadata: ImagePostMetadataSchema, upload_file: UploadFil def get(self, image_id: str) -> ImageSchema: """ - Retrieve an image's metadata with its presigned get url by its ID. + Retrieve an image's metadata with its presigned get download and view urls by its ID. :param image_id: ID of the image to retrieve. - :return: An image's metadata with a presigned get url. + :return: An image's metadata with its presigned get urls. """ image = self._image_repository.get(image_id=image_id) - presigned_url = self._image_store.create_presigned_get(image) - return ImageSchema(**image.model_dump(), url=presigned_url) + (view_url, download_url) = self._image_store.create_presigned_get(image) + return ImageSchema(**image.model_dump(), view_url=view_url, download_url=download_url) def list(self, entity_id: Optional[str] = None, primary: Optional[bool] = None) -> list[ImageMetadataSchema]: """ diff --git a/object_storage_api/stores/image.py b/object_storage_api/stores/image.py index 31ea881..9821844 100644 --- a/object_storage_api/stores/image.py +++ b/object_storage_api/stores/image.py @@ -39,25 +39,38 @@ def upload(self, image_id: str, image_metadata: ImagePostMetadataSchema, upload_ return object_key - def create_presigned_get(self, image: ImageOut) -> str: + def create_presigned_get(self, image: ImageOut) -> tuple[str, str]: """ Generate a presigned URL to share an S3 object. :param image: `ImageOut` model of the image. - :return: Presigned url to get the image. + :return: Presigned urls to view and download the image. """ logger.info("Generating presigned url to get image with object key: %s from the object store", image.object_key) - response = s3_client.generate_presigned_url( - "get_object", - Params={ + + parameters = { + "ClientMethod": "get_object", + "Params": { "Bucket": object_storage_config.bucket_name.get_secret_value(), "Key": image.object_key, "ResponseContentDisposition": f'inline; filename="{image.file_name}"', }, - ExpiresIn=object_storage_config.presigned_url_expiry_seconds, + "ExpiresIn": object_storage_config.presigned_url_expiry_seconds, + } + + view_response = s3_client.generate_presigned_url(**parameters) + + attachment_response = s3_client.generate_presigned_url( + **{ + **parameters, + "Params": { + **parameters["Params"], + "ResponseContentDisposition": f'attachment; filename="{image.file_name}"', + }, + } ) - return response + return (view_response, attachment_response) def delete(self, object_key: str) -> None: """ diff --git a/test/mock_data.py b/test/mock_data.py index 60d4e01..67030df 100644 --- a/test/mock_data.py +++ b/test/mock_data.py @@ -170,5 +170,6 @@ IMAGE_GET_DATA_ALL_VALUES = { **IMAGE_GET_METADATA_DATA_ALL_VALUES, - "url": ANY, + "view_url": ANY, + "download_url": ANY, } diff --git a/test/unit/services/test_image.py b/test/unit/services/test_image.py index 39fd332..b0d5abd 100644 --- a/test/unit/services/test_image.py +++ b/test/unit/services/test_image.py @@ -185,9 +185,14 @@ def mock_get(self) -> None: self._expected_image_out = ImageOut(**ImageIn(**IMAGE_IN_DATA_ALL_VALUES).model_dump()) self.mock_image_repository.get.return_value = self._expected_image_out - self.mock_image_store.create_presigned_get.return_value = "https://fakepresignedurl.co.uk" + self.mock_image_store.create_presigned_get.return_value = ( + "https://fakepresignedurl.co.uk/inline", + "https://fakepresignedurl.co.uk/attachment", + ) self._expected_image = ImageSchema( - **self._expected_image_out.model_dump(), url="https://fakepresignedurl.co.uk" + **self._expected_image_out.model_dump(), + view_url="https://fakepresignedurl.co.uk/inline", + download_url="https://fakepresignedurl.co.uk/attachment", ) def call_get(self, image_id: str) -> None: diff --git a/test/unit/stores/test_image.py b/test/unit/stores/test_image.py index 936ee73..cab1b82 100644 --- a/test/unit/stores/test_image.py +++ b/test/unit/stores/test_image.py @@ -3,7 +3,7 @@ """ from test.mock_data import IMAGE_IN_DATA_ALL_VALUES, IMAGE_POST_METADATA_DATA_ALL_VALUES -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import pytest from bson import ObjectId @@ -118,8 +118,10 @@ class CreatePresignedURLDSL(ImageStoreDSL): """Base class for `create` tests.""" _image_out: ImageOut - _expected_presigned_url: str - _obtained_presigned_url: str + _expected_presigned_view_url: str + _obtained_presigned_view_url: str + _expected_presigned_download_url: str + _obtained_presigned_download_url: str def mock_create_presigned_get(self, image_in_data: dict) -> None: """ @@ -131,8 +133,12 @@ def mock_create_presigned_get(self, image_in_data: dict) -> None: self._image_out = ImageOut(**ImageIn(**image_in_data).model_dump()) # Mock presigned url generation - self._expected_presigned_url = "example_presigned_url" - self.mock_s3_client.generate_presigned_url.return_value = self._expected_presigned_url + self._expected_presigned_view_url = "example_presigned_view_url" + self._expected_presigned_download_url = "example_presigned_downloadurl" + self.mock_s3_client.generate_presigned_url.side_effect = [ + self._expected_presigned_view_url, + self._expected_presigned_download_url, + ] def call_create_presigned_get(self) -> None: """ @@ -140,29 +146,46 @@ def call_create_presigned_get(self) -> None: `mock_create_presigned_get`. """ - self._obtained_presigned_url = self.image_store.create_presigned_get(self._image_out) + (self._obtained_presigned_view_url, self._obtained_presigned_download_url) = ( + self.image_store.create_presigned_get(self._image_out) + ) def check_create_presigned_get_success(self) -> None: """Checks that a prior call to `call_create_presigned_get` worked as expected.""" - self.mock_s3_client.generate_presigned_url.assert_called_once_with( - "get_object", - Params={ + parameters = { + "ClientMethod": "get_object", + "Params": { "Bucket": object_storage_config.bucket_name.get_secret_value(), "Key": self._image_out.object_key, "ResponseContentDisposition": f'inline; filename="{self._image_out.file_name}"', }, - ExpiresIn=object_storage_config.presigned_url_expiry_seconds, - ) - - assert self._obtained_presigned_url == self._expected_presigned_url + "ExpiresIn": object_storage_config.presigned_url_expiry_seconds, + } + + expected_calls = [ + call.generate_presigned_url(**parameters), + call.generate_presigned_url( + **{ + **parameters, + "Params": { + **parameters["Params"], + "ResponseContentDisposition": f'attachment; filename="{self._image_out.file_name}"', + }, + } + ), + ] + self.mock_s3_client.assert_has_calls(expected_calls) + + assert self._obtained_presigned_view_url == self._expected_presigned_view_url + assert self._obtained_presigned_download_url == self._expected_presigned_download_url class TestCreatePresignedURL(CreatePresignedURLDSL): - """Tests for creating a presigned url for an image.""" + """Tests for creating presigned get urls for an image.""" def test_create_presigned_get(self): - """Test creating a presigned url for an image.""" + """Test creating presigned get urls for an image.""" self.mock_create_presigned_get(IMAGE_IN_DATA_ALL_VALUES) self.call_create_presigned_get()