diff --git a/app/template/rest.py b/app/template/rest.py index c03a2c23e..9db0bed57 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -1,14 +1,19 @@ import base64 +from io import BytesIO import botocore +from PyPDF2.utils import PdfReadError from flask import ( Blueprint, current_app, jsonify, request) +from notifications_utils.pdf import extract_page_from_pdf +from notifications_utils.template import SMSMessageTemplate from requests import post as requests_post from app.dao.notifications_dao import get_notification_by_id +from app.dao.services_dao import dao_fetch_service_by_id from app.dao.templates_dao import ( dao_update_template, dao_create_template, @@ -18,16 +23,14 @@ from app.dao.templates_dao import ( dao_get_template_versions, dao_update_template_reply_to, dao_get_template_by_id) -from notifications_utils.template import SMSMessageTemplate -from app.dao.services_dao import dao_fetch_service_by_id -from app.letters.utils import get_letter_pdf -from app.models import SMS_TYPE -from app.notifications.validators import service_has_permission, check_reply_to -from app.schemas import (template_schema, template_history_schema) from app.errors import ( register_errors, InvalidRequest ) +from app.letters.utils import get_letter_pdf +from app.models import SMS_TYPE +from app.notifications.validators import service_has_permission, check_reply_to +from app.schemas import (template_schema, template_history_schema) from app.utils import get_template_instance, get_public_notify_type_text template_blueprint = Blueprint('template', __name__, url_prefix='/service//template') @@ -202,18 +205,30 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file pdf_file = get_letter_pdf(notification) - except botocore.exceptions.ClientError: - current_app.logger.exception( - 'Error getting letter file from S3 notification id {}'.format(notification_id)) - raise InvalidRequest('Error getting letter file from S3 notification id {}'.format(notification_id), - status_code=500) + except botocore.exceptions.ClientError as e: + raise InvalidRequest( + 'Error extracting requested page from PDF file for notification_id {} type {} {}'.format( + notification_id, type(e), e), + status_code=500 + ) content = base64.b64encode(pdf_file).decode('utf-8') if file_type == 'png': - url = '{}/precompiled-preview.png{}'.format( - current_app.config['TEMPLATE_PREVIEW_API_HOST'], - '?page={}'.format(page) if page else '' + + try: + page_number = page if page else "0" + pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page_number) - 1) + content = base64.b64encode(pdf_page).decode('utf-8') + except PdfReadError as e: + raise InvalidRequest( + 'Error extracting requested page from PDF file for notification_id {} type {} {}'.format( + notification_id, type(e), e), + status_code=500 + ) + + url = '{}/precompiled-preview.png'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'] ) content = _get_png_preview(url, content, notification.id, json=False) @@ -262,14 +277,8 @@ def _get_png_preview(url, data, notification_id, json=True): ) if resp.status_code != 200: - current_app.logger.exception( - 'Error generating preview letter for {} \nStatus code: {}\n{}'.format( - notification_id, - resp.status_code, - resp.content - )) raise InvalidRequest( - 'Error generating preview letter for {}\nStatus code: {}\n{}'.format( + 'Error generating preview letter for {} Status code: {} {}'.format( notification_id, resp.status_code, resp.content diff --git a/requirements.txt b/requirements.txt index c08e95a1b..83d7973ed 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,6 +23,6 @@ notifications-python-client==4.7.2 # PaaS awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@25.0.0#egg=notifications-utils==25.0.0 +git+https://github.com/alphagov/notifications-utils.git@25.1.0#egg=notifications-utils==25.1.0 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 6611c772d..14145014e 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -7,6 +7,7 @@ from datetime import datetime, timedelta import botocore import pytest import requests_mock +from PyPDF2.utils import PdfReadError from freezegun import freeze_time from app.models import Template, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, TemplateHistory @@ -945,7 +946,7 @@ def test_preview_letter_template_precompiled_s3_error( 'GetObject' )) - admin_request.get( + request = admin_request.get( 'template.preview_letter_template_by_notification_id', service_id=notification.service_id, notification_id=notification.id, @@ -953,6 +954,10 @@ def test_preview_letter_template_precompiled_s3_error( _expected_status=500 ) + assert request['message'] == "Error extracting requested page from PDF file for notification_id {} type " \ + " An error occurred (403) " \ + "when calling the GetObject operation: Unauthorized".format(notification.id) + def test_preview_letter_template_precompiled_png_file_type( notify_api, @@ -981,6 +986,8 @@ def test_preview_letter_template_precompiled_png_file_type( mock_get_letter_pdf = mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + mocker.patch('app.template.rest.extract_page_from_pdf', return_value=pdf_content) + mock_post = request_mock.post( 'http://localhost/notifications-template-preview/precompiled-preview.png', content=png_content, @@ -1028,6 +1035,8 @@ def test_preview_letter_template_precompiled_png_template_preview_500_error( mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + mocker.patch('app.template.rest.extract_page_from_pdf', return_value=pdf_content) + mock_post = request_mock.post( 'http://localhost/notifications-template-preview/precompiled-preview.png', content=png_content, @@ -1075,6 +1084,8 @@ def test_preview_letter_template_precompiled_png_template_preview_400_error( mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + mocker.patch('app.template.rest.extract_page_from_pdf', return_value=pdf_content) + mock_post = request_mock.post( 'http://localhost/notifications-template-preview/precompiled-preview.png', content=png_content, @@ -1092,3 +1103,52 @@ def test_preview_letter_template_precompiled_png_template_preview_400_error( with pytest.raises(ValueError): mock_post.last_request.json() + + +def test_preview_letter_template_precompiled_png_template_preview_pdf_error( + notify_api, + client, + admin_request, + sample_service, + mocker +): + + template = create_template(sample_service, + template_type='letter', + template_name='Pre-compiled PDF', + subject='Pre-compiled PDF', + hidden=True) + + notification = create_notification(template) + + with set_config_values(notify_api, { + 'TEMPLATE_PREVIEW_API_HOST': 'http://localhost/notifications-template-preview', + 'TEMPLATE_PREVIEW_API_KEY': 'test-key' + }): + with requests_mock.Mocker() as request_mock: + + pdf_content = b'\x00\x01' + png_content = b'\x00\x02' + + mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + + error_message = "PDF Error message" + mocker.patch('app.template.rest.extract_page_from_pdf', side_effect=PdfReadError(error_message)) + + request_mock.post( + 'http://localhost/notifications-template-preview/precompiled-preview.png', + content=png_content, + headers={'X-pdf-page-count': '1'}, + status_code=404 + ) + + request = admin_request.get( + 'template.preview_letter_template_by_notification_id', + service_id=notification.service_id, + notification_id=notification.id, + file_type='png', + _expected_status=500 + ) + + assert request['message'] == "Error extracting requested page from PDF file for notification_id {} type " \ + "{} {}".format(notification.id, type(PdfReadError()), error_message)