From 17183efd34ef45f7132d9ba45daf9e24ff735d94 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Mon, 5 Feb 2024 17:38:25 +0000 Subject: [PATCH] safekeeping --- caseworker/cases/views/main.py | 40 +++++++++++----- core/client.py | 4 +- core/services.py | 6 +++ exporter/organisation/views.py | 25 ++++++++-- .../caseworker/cases/views/test_main.py | 48 ++++++++++++++----- 5 files changed, 92 insertions(+), 31 deletions(-) create mode 100644 core/services.py diff --git a/caseworker/cases/views/main.py b/caseworker/cases/views/main.py index 74dee71d57..2e0439123f 100644 --- a/caseworker/cases/views/main.py +++ b/caseworker/cases/views/main.py @@ -5,15 +5,21 @@ from http import HTTPStatus -from django.conf import settings from django.contrib import messages from django.contrib.messages.views import SuccessMessageMixin -from django.http import Http404 +from django.http import ( + Http404, + StreamingHttpResponse, +) from django.shortcuts import redirect from django.urls import reverse, reverse_lazy from django.utils import timezone from django.utils.functional import cached_property -from django.views.generic import FormView, TemplateView, View +from django.views.generic import ( + FormView, + TemplateView, + View, +) from requests.exceptions import HTTPError @@ -22,7 +28,7 @@ from core.decorators import expect_status from core.exceptions import APIError from core.helpers import get_document_data -from core.file_handler import download_document_from_s3 +from core.services import stream_document from lite_content.lite_internal_frontend import cases from lite_content.lite_internal_frontend.cases import ( @@ -64,7 +70,6 @@ put_next_review_date, reissue_ogl, post_case_documents, - get_document, get_blocking_flags, get_case_sub_statuses, put_case_sub_status, @@ -543,15 +548,24 @@ def post(self, request, **kwargs): ) -class Document(View): - def get(self, request, **kwargs): - document, _ = get_document(request, pk=kwargs["file_pk"]) - document = document["document"] +class Document(LoginRequiredMixin, View): + @expect_status( + HTTPStatus.OK, + "Error downloading document", + "Unexpected error downloading document", + ) + def stream_document(self, request, pk): + return stream_document(request, pk=pk) - return download_document_from_s3( - document["s3_key"], - document["name"], - ) + def get(self, request, **kwargs): + api_response, _ = self.stream_document(request, pk=kwargs["file_pk"]) + response = StreamingHttpResponse(api_response.iter_content()) + for header_to_copy in [ + "Content-Type", + "Content-Disposition", + ]: + response.headers[header_to_copy] = api_response.headers[header_to_copy] + return response class CaseOfficer(SingleFormView): diff --git a/core/client.py b/core/client.py index 8f8c3eadd8..9f46069941 100644 --- a/core/client.py +++ b/core/client.py @@ -77,7 +77,7 @@ def verify_hawk_response(response, sender): ) -def perform_request(method, request, appended_address, data=None): +def perform_request(method, request, appended_address, data=None, stream=False): data = data or {} session = request.requests_session # provided by RequestsSessionMiddleware url = _build_absolute_uri(appended_address.replace(" ", "%20")) @@ -90,7 +90,7 @@ def perform_request(method, request, appended_address, data=None): logger.debug("API request: %s %s %s %s", method, url, headers, data) - response = session.request(method=method, url=url, headers=headers, json=data) + response = session.request(method=method, url=url, headers=headers, json=data, stream=stream) if settings.HAWK_AUTHENTICATION_ENABLED: verify_hawk_response(response=response, sender=sender) diff --git a/core/services.py b/core/services.py new file mode 100644 index 0000000000..f19cb8a3fa --- /dev/null +++ b/core/services.py @@ -0,0 +1,6 @@ +from core import client + + +def stream_document(request, pk): + response = client.get(request, f"/documents/stream/{pk}/", stream=True) + return response, response.status_code diff --git a/exporter/organisation/views.py b/exporter/organisation/views.py index af55f1bf5d..570c945cc5 100644 --- a/exporter/organisation/views.py +++ b/exporter/organisation/views.py @@ -1,3 +1,5 @@ +from http import HTTPStatus + from django.shortcuts import render from django.urls import reverse, reverse_lazy from django.views.generic import FormView, TemplateView, RedirectView, View @@ -7,8 +9,9 @@ from core.auth.views import LoginRequiredMixin from core.constants import OrganisationDocumentType +from core.decorators import expect_status from core.helpers import get_document_data -from core.file_handler import download_document_from_s3 +from core.services import stream_document from exporter.core.constants import Permissions from exporter.core.objects import Tab @@ -61,16 +64,28 @@ class Details(LoginRequiredMixin, OrganisationView): class DocumentOnOrganisation(LoginRequiredMixin, View): + @expect_status( + HTTPStatus.OK, + "Error downloading document", + "Unexpected error downloading document", + ) + def stream_document(self, request, pk): + return stream_document(request, pk=pk) + def get(self, request, pk): organisation_id = str(self.request.session["organisation"]) response = get_document_on_organisation(request=self.request, organisation_id=organisation_id, document_id=pk) document_on_organisation = response.json() document = document_on_organisation["document"] - return download_document_from_s3( - document["s3_key"], - document["name"], - ) + api_response, _ = self.stream_document(request, pk=document["id"]) + response = StreamingHttpResponse(api_response.iter_content()) + for header_to_copy in [ + "Content-Type", + "Content-Disposition", + ]: + response.headers[header_to_copy] = api_response.headers[header_to_copy] + return response class AbstractOrganisationUpload(LoginRequiredMixin, FormView): diff --git a/unit_tests/caseworker/cases/views/test_main.py b/unit_tests/caseworker/cases/views/test_main.py index 88483c488b..ad5826c26d 100644 --- a/unit_tests/caseworker/cases/views/test_main.py +++ b/unit_tests/caseworker/cases/views/test_main.py @@ -1,9 +1,12 @@ +import io +import pytest import uuid from django.http import StreamingHttpResponse from django.urls import reverse from core import client +from core.exceptions import ServiceError def test_document_download( @@ -12,20 +15,14 @@ def test_document_download( queue_pk, data_standard_case, requests_mock, - mock_s3_files, ): - mock_s3_files( - ("123", b"test", {"ContentType": "application/doc"}), - ) - document_id = uuid.uuid4() requests_mock.get( - client._build_absolute_uri(f"/documents/{document_id}"), - json={ - "document": { - "s3_key": "123", - "name": "fakefile.doc", - }, + url=client._build_absolute_uri(f"/documents/stream/{document_id}/"), + body=io.BytesIO(b"test"), + headers={ + "Content-Type": "application/doc", + "Content-Disposition": 'attachment; filename="fakefile.doc"', }, ) @@ -44,3 +41,32 @@ def test_document_download( assert response.headers["Content-Type"] == "application/doc" assert response.headers["Content-Disposition"] == 'attachment; filename="fakefile.doc"' assert b"".join(response.streaming_content) == b"test" + + +def test_document_download_no_response( + authorized_client, + mock_queue, + queue_pk, + data_standard_case, + requests_mock, +): + document_id = uuid.uuid4() + requests_mock.get( + url=client._build_absolute_uri(f"/documents/stream/{document_id}/"), + status_code=404, + ) + + url = reverse( + "cases:document", + kwargs={ + "queue_pk": queue_pk, + "pk": data_standard_case["case"]["id"], + "file_pk": document_id, + }, + ) + + with pytest.raises(ServiceError) as ex: + authorized_client.get(url) + + assert ex.value.status_code == 404 + assert ex.value.user_message == "Unexpected error downloading document"