Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect client to download media directly from Amazon S3 / Azure Storage #2713

Merged
merged 21 commits into from
Oct 14, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
download export directly from s3
kelvin-muchiri committed Oct 14, 2024
commit b52a5f214ad9c4b6d86bca5e170c203d293b4fea
54 changes: 49 additions & 5 deletions onadata/apps/api/tests/viewsets/test_export_viewset.py
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@
"""
import os
from tempfile import NamedTemporaryFile
from unittest.mock import MagicMock, patch

from django.conf import settings
from django.utils.dateparse import parse_datetime
@@ -38,18 +39,18 @@ def test_export_response(self):
"""
self._create_user_and_login()
self._publish_transportation_form()
temp_dir = settings.MEDIA_ROOT
# Create the 'exports' directory inside MEDIA_ROOT if it does not exist
temp_dir = os.path.join(settings.MEDIA_ROOT, "exports")
os.makedirs(temp_dir, exist_ok=True)
dummy_export_file = NamedTemporaryFile(suffix=".xlsx", dir=temp_dir)
filename = os.path.basename(dummy_export_file.name)
filedir = os.path.dirname(dummy_export_file.name)
export = Export.objects.create(
xform=self.xform, filename=filename, filedir=filedir
xform=self.xform, filename=filename, filedir="exports"
)
export.save()
request = self.factory.get("/export")
force_authenticate(request, user=self.user)
response = self.view(request, pk=export.pk)
self.assertIn(filename, response.get("Content-Disposition"))
self.assertIn(export.filename, response.get("Content-Disposition"))

def test_export_formats_present(self):
"""
@@ -622,3 +623,46 @@ def test_export_are_downloadable_to_all_users_when_public_form(self):
request = self.factory.get("/export", **alices_extra)
response = self.view(request, pk=export.pk)
self.assertEqual(response.status_code, 200)

@patch("onadata.libs.utils.image_tools.get_storage_class")
@patch("onadata.libs.utils.image_tools.boto3.client")
def test_download_from_s3(self, mock_presigned_urls, mock_get_storage_class):
"""Export is downloaded from Amazon S3"""
expected_url = (
"https://testing.s3.amazonaws.com/bob/exports/"
"trees/csv/trees_2024_06_21_07_47_24_026998.csv?"
"response-content-disposition=attachment%3Bfilename%trees.csv&"
"response-content-type=application%2Foctet-stream&"
"AWSAccessKeyId=AKIAJ3XYHHBIJDL7GY7A"
"&Signature=aGhiK%2BLFVeWm%2Fmg3S5zc05g8%3D&Expires=1615554960"
)
mock_presigned_urls().generate_presigned_url = MagicMock(
return_value=expected_url
)
mock_get_storage_class()().bucket.name = "onadata"
self._create_user_and_login()
self._publish_transportation_form()
# Create the 'exports' directory inside MEDIA_ROOT if it does not exist
temp_dir = os.path.join(settings.MEDIA_ROOT, "exports")
os.makedirs(temp_dir, exist_ok=True)
dummy_export_file = NamedTemporaryFile(suffix=".xlsx", dir=temp_dir)
filename = os.path.basename(dummy_export_file.name)
export = Export.objects.create(
xform=self.xform, filename=filename, filedir="exports"
)
request = self.factory.get("/export")
force_authenticate(request, user=self.user)
response = self.view(request, pk=export.pk)
self.assertEqual(response.status_code, 302, response.url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the 3rd argument to assertEqual do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrankApiyo I copied the assertion from a previous implementation. I will remove it since the 3rd argument should be the message displayed if the assertion fails.

self.assertEqual(response.url, expected_url)
self.assertTrue(mock_presigned_urls.called)
mock_presigned_urls().generate_presigned_url.assert_called_with(
"get_object",
Params={
"Bucket": "onadata",
"Key": export.filepath,
"ResponseContentDisposition": f'attachment; filename="{filename}"',
"ResponseContentType": "application/octet-stream",
},
ExpiresIn=3600,
)
16 changes: 7 additions & 9 deletions onadata/apps/api/viewsets/export_viewset.py
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@
from onadata.libs.authentication import TempTokenURLParameterAuthentication
from onadata.libs.renderers import renderers
from onadata.libs.serializers.export_serializer import ExportSerializer
from onadata.libs.utils.logger_tools import response_with_mimetype_and_name
from onadata.libs.utils.image_tools import generate_media_download_url


# pylint: disable=too-many-ancestors
@@ -47,13 +47,11 @@ class ExportViewSet(DestroyModelMixin, ReadOnlyModelViewSet):

def retrieve(self, request, *args, **kwargs):
export = self.get_object()
filename, extension = os.path.splitext(export.filename)
_, extension = os.path.splitext(export.filename)
extension = extension[1:]
mimetype = f"application/{Export.EXPORT_MIMES[extension]}"

return response_with_mimetype_and_name(
Export.EXPORT_MIMES[extension],
filename,
extension=extension,
file_path=export.filepath,
show_date=False,
)
if Export.EXPORT_MIMES[extension] == "csv":
mimetype = "text/csv"

return generate_media_download_url(export.filepath, mimetype, export.filename)
4 changes: 4 additions & 0 deletions onadata/libs/utils/api_export_tools.py
Original file line number Diff line number Diff line change
@@ -714,4 +714,8 @@ def _new_export():
if Export.EXPORT_MIMES[ext] == "csv":
mimetype = "text/csv"

import ipdb

ipdb.set_trace()

return generate_media_download_url(export.filepath, mimetype, f"{filename}.{ext}")