From 04048aa220e696cf0d385f8882a264fc270f4ec1 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Fri, 9 Mar 2018 15:50:43 +0000 Subject: [PATCH] Updated the notification template endpoint to extract the pdf page before sending it to template preview. This stops the whole pdf file being sent to template preview for each page which is really inefficient on network traffic and memory usage. * Added logic to the endpoint to extract the specific page requested * Updated tests to add a mock for the new call to utils * Added a new test case for exceptions in the PDF extraction process --- app/template/rest.py | 21 +++++++++++-- requirements.txt | 2 +- tests/app/template/test_rest.py | 52 +++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/app/template/rest.py b/app/template/rest.py index c03a2c23e..960ce4e60 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -1,11 +1,14 @@ 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 requests import post as requests_post from app.dao.notifications_dao import get_notification_by_id @@ -211,9 +214,21 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file 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: + current_app.logger.exception( + 'Error extracting requested page from PDF file for notification_id {}'.format(notification_id)) + raise InvalidRequest( + 'Error extracting requested page from PDF file for notification_id {}'.format(notification_id), + 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) diff --git a/requirements.txt b/requirements.txt index 74dc825f1..11bc6c0e4 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..41d0c0b7f 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 @@ -981,6 +982,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 +1031,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 +1080,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 +1099,48 @@ 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) + + mocker.patch('app.template.rest.extract_page_from_pdf', side_effect=PdfReadError()) + + request_mock.post( + 'http://localhost/notifications-template-preview/precompiled-preview.png', + content=png_content, + headers={'X-pdf-page-count': '1'}, + status_code=404 + ) + + 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 + )