diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py new file mode 100644 index 000000000..69e18e645 --- /dev/null +++ b/app/celery/letters_pdf_tasks.py @@ -0,0 +1,68 @@ +from flask import current_app +from requests import ( + post as requests_post, + RequestException +) + +from botocore.exceptions import ClientError as BotoClientError +from sqlalchemy.orm.exc import NoResultFound + +from app import notify_celery +from app.aws import s3 +from app.config import QueueNames +from app.dao.notifications_dao import get_notification_by_id, update_notification_status_by_id +from app.statsd_decorators import statsd + + +@notify_celery.task(bind=True, name="create-letters-pdf", max_retries=15, default_retry_delay=300) +@statsd(namespace="tasks") +def create_letters_pdf(self, notification_id): + try: + notification = get_notification_by_id(notification_id) + if not notification: + raise NoResultFound() + + pdf_data = get_letters_pdf( + notification.template, + contact_block=notification.reply_to_text, + org_id=notification.service.dvla_organisation.id, + values=notification.personalisation + ) + current_app.logger.info("PDF Letter {} reference {} created at {}, {} bytes".format( + notification.id, notification.reference, notification.created_at, len(pdf_data))) + s3.upload_letters_pdf(reference=notification.reference, crown=True, filedata=pdf_data) + except (RequestException, BotoClientError): + try: + current_app.logger.exception( + "Letters PDF notification creation for id: {} failed".format(notification_id) + ) + self.retry(queue=QueueNames.RETRY) + except self.MaxRetriesExceededError: + current_app.logger.exception( + "RETRY FAILED: task create_letters_pdf failed for notification {}".format(notification_id), + ) + update_notification_status_by_id(notification_id, 'technical-failure') + + +def get_letters_pdf(template, contact_block, org_id, values): + template_for_letter_print = { + "subject": template.subject, + "content": template.content + } + + data = { + 'letter_contact_block': contact_block, + 'template': template_for_letter_print, + 'values': values, + 'dvla_org_id': org_id, + } + resp = requests_post( + '{}/print.pdf'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'] + ), + json=data, + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) + resp.raise_for_status() + + return resp.content diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 9b4d2e398..f1260a6f0 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -4,7 +4,6 @@ from collections import namedtuple from celery.signals import worker_process_shutdown from flask import current_app -import requests from notifications_utils.recipients import ( RecipientCSV @@ -20,7 +19,6 @@ from requests import ( RequestException ) from sqlalchemy.exc import SQLAlchemyError -from sqlalchemy.orm.exc import NoResultFound from botocore.exceptions import ClientError as BotoClientError from app import ( @@ -32,6 +30,7 @@ from app import ( ) from app.aws import s3 from app.celery import provider_tasks +from app.celery import letters_pdf_tasks from app.celery.service_callback_tasks import send_delivery_status_to_service from app.config import QueueNames from app.dao.inbound_sms_dao import dao_get_inbound_sms_by_id @@ -48,7 +47,6 @@ from app.dao.notifications_dao import ( dao_update_notifications_by_reference, dao_get_last_notification_added_for_job_id, dao_get_notifications_by_references, - update_notification_status_by_id ) from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service @@ -127,7 +125,7 @@ def process_job(job_id): def job_complete(job, service, template_type, resumed=False, start=None): if ( template_type == LETTER_TYPE and - 'letters_as_pdf' not in [p.permission for p in service.permissions] + not service.has_permission('letters_as_pdf') ): if service.research_mode: update_job_to_sent_to_dvla.apply_async([str(job.id)], queue=QueueNames.RESEARCH_MODE) @@ -320,9 +318,9 @@ def save_letter( ) if ( - 'letters_as_pdf' in [p.permission for p in service.permissions] and not service.research_mode + service.has_permission('letters_as_pdf') and not service.research_mode ): - create_letters_pdf.apply_async( + letters_pdf_tasks.create_letters_pdf.apply_async( [str(saved_notification.id)], queue=QueueNames.CREATE_LETTERS_PDF ) @@ -596,57 +594,3 @@ def process_incomplete_job(job_id): process_row(row_number, recipient, personalisation, template, job, job.service) job_complete(job, job.service, template.template_type, resumed=True) - - -@notify_celery.task(bind=True, name="create-letters-pdf", max_retries=15, default_retry_delay=300) -@statsd(namespace="tasks") -def create_letters_pdf(self, notification_id): - try: - notification = get_notification_by_id(notification_id) - if not notification: - raise NoResultFound() - - pdf_data = get_letters_pdf( - notification.template, - contact_block=notification.reply_to_text, - org_id=notification.service.dvla_organisation.id, - values=notification.personalisation - ) - current_app.logger.info("PDF Letter {} reference {} created at {}, {} bytes".format( - notification.id, notification.reference, notification.created_at, len(pdf_data))) - s3.upload_letters_pdf(reference=notification.reference, crown=True, filedata=pdf_data) - except (RequestException, BotoClientError): - try: - current_app.logger.exception( - "Letters PDF notification creation for id: {} failed".format(notification_id) - ) - self.retry(queue=QueueNames.RETRY) - except self.MaxRetriesExceededError: - current_app.logger.exception( - "RETRY FAILED: task create_letters_pdf failed for notification {}".format(notification_id), - ) - update_notification_status_by_id(notification_id, 'technical-failure') - - -def get_letters_pdf(template, contact_block, org_id, values=None): - template_for_letter_print = { - "subject": template.subject, - "content": template.content - } - - data = { - 'letter_contact_block': contact_block, - 'template': template_for_letter_print, - 'values': values, - 'dvla_org_id': org_id, - } - resp = requests.post( - '{}/print.pdf'.format( - current_app.config['TEMPLATE_PREVIEW_API_HOST'] - ), - json=data, - headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} - ) - resp.raise_for_status() - - return resp.content diff --git a/app/models.py b/app/models.py index bd016b4d0..2f196c7a5 100644 --- a/app/models.py +++ b/app/models.py @@ -283,6 +283,9 @@ class Service(db.Model, Versioned): default_letter_contact = [x for x in self.letter_contacts if x.is_default] return default_letter_contact[0].contact_block if default_letter_contact else None + def has_permission(self, permission): + return permission in [p.permission for p in self.permissions] + class AnnualBilling(db.Model): __tablename__ = "annual_billing" diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py new file mode 100644 index 000000000..27f727571 --- /dev/null +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -0,0 +1,99 @@ +import pytest +import requests_mock + +from botocore.exceptions import ClientError +from celery.exceptions import MaxRetriesExceededError +from requests import RequestException +from sqlalchemy.orm.exc import NoResultFound + +from app.celery.letters_pdf_tasks import ( + create_letters_pdf, + get_letters_pdf, +) + +from tests.conftest import set_config_values + + +def test_should_have_decorated_tasks_functions(): + assert create_letters_pdf.__wrapped__.__name__ == 'create_letters_pdf' + + +@pytest.mark.parametrize('personalisation', [{'name': 'test'}, None]) +def test_get_letters_pdf_calls_notifications_template_preview_service_correctly( + notify_api, mocker, client, sample_letter_template, personalisation): + contact_block = 'Mr Foo,\n1 Test Street,\nLondon\nN1' + dvla_org_id = '002' + + 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: + mock_post = request_mock.post( + 'http://localhost/notifications-template-preview/print.pdf', content=b'\x00\x01', status_code=200) + + get_letters_pdf( + sample_letter_template, contact_block=contact_block, org_id=dvla_org_id, values=personalisation) + + assert mock_post.last_request.json() == { + 'values': personalisation, + 'letter_contact_block': contact_block, + 'dvla_org_id': dvla_org_id, + 'template': { + 'subject': sample_letter_template.subject, + 'content': sample_letter_template.content + } + } + + +def test_create_letters_pdf_calls_upload_letters_pdf(mocker, sample_letter_notification): + mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=b'\x00\x01') + mock_s3 = mocker.patch('app.celery.tasks.s3.upload_letters_pdf') + + create_letters_pdf(sample_letter_notification.id) + + mock_s3.assert_called_with( + reference=sample_letter_notification.reference, + crown=True, + filedata=b'\x00\x01' + ) + + +def test_create_letters_pdf_non_existent_notification(notify_api, mocker, fake_uuid): + with pytest.raises(expected_exception=NoResultFound): + create_letters_pdf(fake_uuid) + + +def test_create_letters_pdf_handles_request_errors(mocker, sample_letter_notification): + mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', side_effect=RequestException) + mock_retry = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.retry') + + create_letters_pdf(sample_letter_notification.id) + + assert mock_get_letters_pdf.called + assert mock_retry.called + + +def test_create_letters_pdf_handles_s3_errors(mocker, sample_letter_notification): + mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf') + mock_s3 = mocker.patch('app.celery.tasks.s3.upload_letters_pdf', side_effect=ClientError({}, 'operation_name')) + mock_retry = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.retry') + + create_letters_pdf(sample_letter_notification.id) + + assert mock_s3.called + assert mock_retry.called + + +def test_create_letters_pdf_sets_technical_failure_max_retries(mocker, sample_letter_notification): + mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', side_effect=RequestException) + mock_retry = mocker.patch( + 'app.celery.letters_pdf_tasks.create_letters_pdf.retry', side_effect=MaxRetriesExceededError) + mock_update_noti = mocker.patch('app.celery.letters_pdf_tasks.update_notification_status_by_id') + + create_letters_pdf(sample_letter_notification.id) + + assert mock_get_letters_pdf.called + assert mock_retry.called + assert mock_update_noti.called + mock_update_noti.assert_called_once_with(sample_letter_notification.id, 'technical-failure') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 7af5c0a06..ae892291d 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -9,9 +9,8 @@ from flask import current_app from freezegun import freeze_time from requests import RequestException from sqlalchemy.exc import SQLAlchemyError -from celery.exceptions import Retry, MaxRetriesExceededError +from celery.exceptions import Retry from botocore.exceptions import ClientError -from sqlalchemy.orm.exc import NoResultFound from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate, LetterDVLATemplate from app import (encryption, DATETIME_FORMAT) @@ -21,8 +20,6 @@ from app.celery.scheduled_tasks import check_job_status from app.celery.tasks import ( build_dvla_file, create_dvla_file_contents_for_job, - create_letters_pdf, - get_letters_pdf, job_complete, process_job, process_row, @@ -51,7 +48,6 @@ from app.models import ( SMS_TYPE ) -from tests.conftest import set_config_values from tests.app import load_example_csv from tests.app.conftest import ( sample_service as create_sample_service, @@ -99,7 +95,6 @@ def test_should_have_decorated_tasks_functions(): assert save_sms.__wrapped__.__name__ == 'save_sms' assert save_email.__wrapped__.__name__ == 'save_email' assert save_letter.__wrapped__.__name__ == 'save_letter' - assert create_letters_pdf.__wrapped__.__name__ == 'create_letters_pdf' @pytest.fixture @@ -1048,7 +1043,7 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session): def test_save_letter_saves_letter_calls_create_letters_as_pdf_with_letters_as_pdf_permission( mocker, notify_db_session, sample_letter_job): service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') - mock_create_letters_pdf = mocker.patch('app.celery.tasks.create_letters_pdf.apply_async') + mock_create_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') personalisation = { 'addressline1': 'Foo', @@ -1505,83 +1500,3 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template): assert mock_build_dvla.called assert mock_letter_saver.call_count == 8 - - -@pytest.mark.parametrize('personalisation', [{'name': 'test'}, None]) -def test_get_letters_pdf_calls_notifications_template_preview_service_correctly( - notify_api, mocker, client, sample_letter_template, personalisation): - contact_block = 'Mr Foo,\n1 Test Street,\nLondon\nN1' - dvla_org_id = '002' - - 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: - mock_post = request_mock.post( - 'http://localhost/notifications-template-preview/print.pdf', content=b'\x00\x01', status_code=200) - - get_letters_pdf( - sample_letter_template, contact_block=contact_block, org_id=dvla_org_id, values=personalisation) - - assert mock_post.last_request.json() == { - 'values': personalisation, - 'letter_contact_block': contact_block, - 'dvla_org_id': dvla_org_id, - 'template': { - 'subject': sample_letter_template.subject, - 'content': sample_letter_template.content - } - } - - -def test_create_letters_pdf_calls_upload_letters_pdf(mocker, sample_letter_notification): - mocker.patch('app.celery.tasks.get_letters_pdf', return_value=b'\x00\x01') - mock_s3 = mocker.patch('app.celery.tasks.s3.upload_letters_pdf') - - create_letters_pdf(sample_letter_notification.id) - - mock_s3.assert_called_with( - reference=sample_letter_notification.reference, - crown=True, - filedata=b'\x00\x01' - ) - - -def test_create_letters_pdf_non_existent_notification(notify_api, mocker, fake_uuid): - with pytest.raises(expected_exception=NoResultFound): - create_letters_pdf(fake_uuid) - - -def test_create_letters_pdf_handles_request_errors(mocker, sample_letter_notification): - mock_get_letters_pdf = mocker.patch('app.celery.tasks.get_letters_pdf', side_effect=RequestException) - mock_retry = mocker.patch('app.celery.tasks.create_letters_pdf.retry') - - create_letters_pdf(sample_letter_notification.id) - - assert mock_get_letters_pdf.called - assert mock_retry.called - - -def test_create_letters_pdf_handles_s3_errors(mocker, sample_letter_notification): - mocker.patch('app.celery.tasks.get_letters_pdf') - mock_s3 = mocker.patch('app.celery.tasks.s3.upload_letters_pdf', side_effect=ClientError({}, 'operation_name')) - mock_retry = mocker.patch('app.celery.tasks.create_letters_pdf.retry') - - create_letters_pdf(sample_letter_notification.id) - - assert mock_s3.called - assert mock_retry.called - - -def test_create_letters_pdf_sets_technical_failure_max_retries(mocker, sample_letter_notification): - mock_get_letters_pdf = mocker.patch('app.celery.tasks.get_letters_pdf', side_effect=RequestException) - mock_retry = mocker.patch('app.celery.tasks.create_letters_pdf.retry', side_effect=MaxRetriesExceededError) - mock_update_noti = mocker.patch('app.celery.tasks.update_notification_status_by_id') - - create_letters_pdf(sample_letter_notification.id) - - assert mock_get_letters_pdf.called - assert mock_retry.called - assert mock_update_noti.called - mock_update_noti.assert_called_once_with(sample_letter_notification.id, 'technical-failure')