Modify tasks for getting letter pdf and updating billable units

So that they talk with new template preview task for pdf creation

app/celery/letters_pdf_tasks.py

@@ -1,15 +1,10 @@
-import math
 from datetime import datetime, timedelta
 from hashlib import sha512
 from base64 import urlsafe_b64encode
 
 from botocore.exceptions import ClientError as BotoClientError
 from flask import current_app
-from requests import (
+
-    post as requests_post,
-    RequestException
-)
-from celery.exceptions import MaxRetriesExceededError
 from notifications_utils.statsd_decorators import statsd
 from notifications_utils.letter_timings import LETTER_PROCESSING_DEADLINE
 from notifications_utils.timezones import convert_utc_to_bst
@@ -31,7 +26,6 @@ from app.exceptions import NotificationTechnicalFailureException
 from app.letters.utils import (
     get_billable_units_for_letter_page_count,
     get_reference_from_filename,
-    upload_letter_pdf,
     ScanErrorType,
     move_failed_pdf,
     move_sanitised_letter_to_test_or_live_pdf_bucket,
@@ -57,14 +51,55 @@ from app.cronitor import cronitor
 def create_letters_pdf(self, notification_id):
     try:
         notification = get_notification_by_id(notification_id, _raise=True)
-        pdf_data, billable_units = get_letters_pdf(
-            notification.template,
-            contact_block=notification.reply_to_text,
-            filename=notification.service.letter_branding and notification.service.letter_branding.filename,
-            values=notification.personalisation
-        )
 
-        upload_letter_pdf(notification, pdf_data)
+        letter_filename = get_letter_pdf_filename(
+            reference=notification.reference,
+            crown=notification.service.crown,
+            sending_date=notification.created_at,
+            dont_use_sending_date=notification.key_type == KEY_TYPE_TEST,
+            postage=notification.postage
+        )
+        letter_data = {
+            'letter_contact_block': notification.reply_to_text,
+            'template': {
+                "subject": notification.template.subject,
+                "content": notification.template.content,
+                "template_type": notification.template.template_type
+            },
+            'values': notification.personalisation,
+            'logo_filename': notification.service.letter_branding and notification.service.letter_branding.filename,
+            'letter_filename': letter_filename,
+            "notification_id": str(notification_id),
+            'key_type': notification.key_type
+        }
+
+        encrypted_data = encryption.encrypt(letter_data)
+
+        notify_celery.send_task(
+            name=TaskNames.CREATE_LETTER_PDF,
+            args=(encrypted_data,),
+            queue=QueueNames.SANITISE_LETTERS
+        )
+    except Exception:
+        try:
+            current_app.logger.exception(
+                "RETRY: calling create-letter-pdf task for notification {} failed".format(notification_id)
+            )
+            self.retry(queue=QueueNames.RETRY)
+        except self.MaxRetriesExceededError:
+            message = "RETRY FAILED: Max retries reached. " \
+                      "The task create-letter-pdf failed for notification {}. " \
+                      "Notification has been updated to technical-failure".format(notification_id)
+            update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
+            raise NotificationTechnicalFailureException(message)
+
+
+@notify_celery.task(bind=True, name="update-billable-units-for-letter", max_retries=15, default_retry_delay=300)
+@statsd(namespace="tasks")
+def update_billable_units_for_letter(self, notification_id, page_count):
+    notification = get_notification_by_id(notification_id, _raise=True)
+
+    billable_units = get_billable_units_for_letter_page_count(page_count)
 
     if notification.key_type != KEY_TYPE_TEST:
         notification.billable_units = billable_units
@@ -74,46 +109,6 @@ def create_letters_pdf(self, notification_id):
         'Letter notification reference {reference}: billable units set to {billable_units}'.format(
             reference=str(notification.reference), billable_units=billable_units))
 
-    except (RequestException, BotoClientError):
-        try:
-            current_app.logger.exception(
-                "Letters PDF notification creation for id: {} failed".format(notification_id)
-            )
-            self.retry(queue=QueueNames.RETRY)
-        except MaxRetriesExceededError:
-            current_app.logger.error(
-                "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, filename, values):
-    template_for_letter_print = {
-        "subject": template.subject,
-        "content": template.content,
-        "template_type": template.template_type
-    }
-
-    data = {
-        'letter_contact_block': contact_block,
-        'template': template_for_letter_print,
-        'values': values,
-        'filename': filename,
-    }
-    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()
-
-    pages_per_sheet = 2
-    billable_units = math.ceil(int(resp.headers.get("X-pdf-page-count", 0)) / pages_per_sheet)
-
-    return resp.content, billable_units
-
 
 @notify_celery.task(name='collate-letter-pdfs-to-be-sent')
 @cronitor("collate-letter-pdfs-to-be-sent")

app/config.py

@@ -59,6 +59,7 @@ class TaskNames(object):
     ZIP_AND_SEND_LETTER_PDFS = 'zip-and-send-letter-pdfs'
     SCAN_FILE = 'scan-file'
     SANITISE_LETTER = 'sanitise-and-upload-letter'
+    CREATE_LETTER_PDF = 'create-letter-pdf'
 
 
 class Config(object):

tests/app/celery/test_letters_pdf_tasks.py

@@ -6,10 +6,8 @@ from moto import mock_s3
 from flask import current_app
 from freezegun import freeze_time
 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 import encryption
@@ -17,7 +15,6 @@ from app.errors import VirusScanError
 from app.exceptions import NotificationTechnicalFailureException
 from app.celery.letters_pdf_tasks import (
     create_letters_pdf,
-    get_letters_pdf,
     collate_letter_pdfs_to_be_sent,
     get_key_and_size_of_letters_to_be_sent_to_print,
     group_letters,
@@ -26,6 +23,7 @@ from app.celery.letters_pdf_tasks import (
     process_virus_scan_error,
     replay_letters_in_error,
     sanitise_letter,
+    update_billable_units_for_letter,
     _move_invalid_letter_and_update_status,
 )
 from app.config import QueueNames, TaskNames
@@ -58,105 +56,32 @@ def test_should_have_decorated_tasks_functions():
     assert process_sanitised_letter.__wrapped__.__name__ == 'process_sanitised_letter'
 
 
-@pytest.mark.parametrize('personalisation', [{'name': 'test'}, None])
+def test_create_letters_pdf(mocker, sample_letter_notification):
-def test_get_letters_pdf_calls_notifications_template_preview_service_correctly(
+    mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task')
-        notify_api, mocker, client, sample_letter_template, personalisation):
+    mocker.patch('app.celery.letters_pdf_tasks.get_letter_pdf_filename', return_value='LETTER.PDF')
-    contact_block = 'Mr Foo,\n1 Test Street,\nLondon\nN1'
-    filename = 'opg'
-
-    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,
-                filename=filename,
-                values=personalisation)
-
-    assert mock_post.last_request.json() == {
-        'values': personalisation,
-        'letter_contact_block': contact_block,
-        'filename': filename,
-        'template': {
-            'subject': sample_letter_template.subject,
-            'content': sample_letter_template.content,
-            'template_type': sample_letter_template.template_type
-        }
-    }
-
-
-@pytest.mark.parametrize('page_count,expected_billable_units', [
-    ('1', 1),
-    ('2', 1),
-    ('3', 2)
-])
-def test_get_letters_pdf_calculates_billing_units(
-        notify_api, mocker, client, sample_letter_template, page_count, expected_billable_units):
-    contact_block = 'Mr Foo,\n1 Test Street,\nLondon\nN1'
-    filename = 'opg'
-
-    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:
-            request_mock.post(
-                'http://localhost/notifications-template-preview/print.pdf',
-                content=b'\x00\x01',
-                headers={'X-pdf-page-count': page_count},
-                status_code=200
-            )
-
-            _, billable_units = get_letters_pdf(
-                sample_letter_template, contact_block=contact_block, filename=filename, values=None)
-
-    assert billable_units == expected_billable_units
-
-
-@freeze_time("2017-12-04 17:31:00")
-def test_create_letters_pdf_calls_s3upload(mocker, sample_letter_template):
-    mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=(b'\x00\x01', '1'))
-    mock_s3 = mocker.patch('app.letters.utils.s3upload')
-    notification = create_notification(template=sample_letter_template, reference='FOO', key_type='normal')
-
-    create_letters_pdf(notification.id)
-
-    mock_s3.assert_called_with(
-        bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'],
-        file_location='2017-12-05/NOTIFY.FOO.D.2.C.C.20171204173100.PDF',
-        filedata=b'\x00\x01',
-        region=current_app.config['AWS_REGION']
-    )
-
-
-@freeze_time("2017-12-04 17:31:00")
-def test_create_letters_pdf_calls_s3upload_for_test_letters(mocker, sample_letter_template):
-    mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=(b'\x00\x01', '1'))
-    mock_s3 = mocker.patch('app.letters.utils.s3upload')
-    notification = create_notification(template=sample_letter_template, reference='FOO', key_type='test')
-
-    create_letters_pdf(notification.id)
-
-    mock_s3.assert_called_with(
-        bucket_name=current_app.config['TEST_LETTERS_BUCKET_NAME'],
-        file_location='NOTIFY.FOO.D.2.C.C.20171204173100.PDF',
-        filedata=b'\x00\x01',
-        region=current_app.config['AWS_REGION']
-    )
-
-
-def test_create_letters_pdf_sets_billable_units(mocker, sample_letter_notification):
-    mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=(b'\x00\x01', 1))
-    mocker.patch('app.letters.utils.s3upload')
-
     create_letters_pdf(sample_letter_notification.id)
-    noti = Notification.query.filter(Notification.reference == sample_letter_notification.reference).one()
+
-    assert noti.billable_units == 1
+    letter_data = {
+        'letter_contact_block': sample_letter_notification.reply_to_text,
+        'template': {
+            "subject": sample_letter_notification.template.subject,
+            "content": sample_letter_notification.template.content,
+            "template_type": sample_letter_notification.template.template_type
+        },
+        'values': sample_letter_notification.personalisation,
+        'logo_filename': None,  # no logo
+        'letter_filename': 'LETTER.PDF',
+        "notification_id": str(sample_letter_notification.id),
+        'key_type': sample_letter_notification.key_type
+    }
+
+    encrypted_data = encryption.encrypt(letter_data)
+
+    mock_celery.assert_called_once_with(
+        name=TaskNames.CREATE_LETTER_PDF,
+        args=(encrypted_data,),
+        queue=QueueNames.SANITISE_LETTERS
+    )
 
 
 def test_create_letters_pdf_non_existent_notification(notify_api, mocker, fake_uuid):
@@ -164,82 +89,71 @@ def test_create_letters_pdf_non_existent_notification(notify_api, mocker, fake_u
         create_letters_pdf(fake_uuid)
 
 
-def test_create_letters_pdf_handles_request_errors(mocker, sample_letter_notification):
+def test_create_letters_pdf_retries_upon_error(mocker, sample_letter_notification):
-    mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', side_effect=RequestException)
+    mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task', side_effect=Exception())
+    mocker.patch('app.celery.letters_pdf_tasks.get_letter_pdf_filename', return_value='LETTER.PDF')
     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_celery.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', return_value=(b'\x00\x01', 1))
-    error_response = {
-        'Error': {
-            'Code': 'InvalidParameterValue',
-            'Message': 'some error message from amazon',
-            'Type': 'Sender'
-        }
-    }
-    mock_s3 = mocker.patch('app.letters.utils.s3upload', side_effect=ClientError(error_response, '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_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task', side_effect=Exception())
+    mocker.patch('app.celery.letters_pdf_tasks.get_letter_pdf_filename', return_value='LETTER.PDF')
     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')
 
+    with pytest.raises(NotificationTechnicalFailureException):
         create_letters_pdf(sample_letter_notification.id)
 
-    assert mock_get_letters_pdf.called
+    assert mock_celery.called
     assert mock_retry.called
     mock_update_noti.assert_called_once_with(sample_letter_notification.id, 'technical-failure')
 
 
-def test_create_letters_gets_the_right_logo_when_service_has_no_logo(
-        notify_api, mocker, sample_letter_notification
-):
-    mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=(b'\x00\x01', 1))
-    mocker.patch('app.letters.utils.s3upload')
-    mocker.patch('app.celery.letters_pdf_tasks.update_notification_status_by_id')
-
-    create_letters_pdf(sample_letter_notification.id)
-    mock_get_letters_pdf.assert_called_once_with(
-        sample_letter_notification.template,
-        contact_block=sample_letter_notification.reply_to_text,
-        filename=None,
-        values=sample_letter_notification.personalisation
-    )
-
-
 # We only need this while we are migrating to the new letter_branding model
 def test_create_letters_gets_the_right_logo_when_service_has_letter_branding_logo(
         notify_api, mocker, sample_letter_notification
 ):
     letter_branding = create_letter_branding(name='test brand', filename='test-brand')
     sample_letter_notification.service.letter_branding = letter_branding
-    mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=(b'\x00\x01', 1))
+    mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task')
-    mocker.patch('app.letters.utils.s3upload')
+    mocker.patch('app.celery.letters_pdf_tasks.get_letter_pdf_filename', return_value='LETTER.PDF')
-    mocker.patch('app.celery.letters_pdf_tasks.update_notification_status_by_id')
-
     create_letters_pdf(sample_letter_notification.id)
-    mock_get_letters_pdf.assert_called_once_with(
+
-        sample_letter_notification.template,
+    letter_data = {
-        contact_block=sample_letter_notification.reply_to_text,
+        'letter_contact_block': sample_letter_notification.reply_to_text,
-        filename=sample_letter_notification.service.letter_branding.filename,
+        'template': {
-        values=sample_letter_notification.personalisation
+            "subject": sample_letter_notification.template.subject,
+            "content": sample_letter_notification.template.content,
+            "template_type": sample_letter_notification.template.template_type
+        },
+        'values': sample_letter_notification.personalisation,
+        'logo_filename': sample_letter_notification.service.letter_branding.filename,
+        'letter_filename': 'LETTER.PDF',
+        "notification_id": str(sample_letter_notification.id),
+        'key_type': sample_letter_notification.key_type
+    }
+
+    encrypted_data = encryption.encrypt(letter_data)
+
+    mock_celery.assert_called_once_with(
+        name=TaskNames.CREATE_LETTER_PDF,
+        args=(encrypted_data,),
+        queue=QueueNames.SANITISE_LETTERS
     )
 
 
+def test_update_billable_units_for_letter(mocker, sample_letter_notification):
+    update_billable_units_for_letter(sample_letter_notification.id, 2)
+    noti = Notification.query.filter(Notification.reference == sample_letter_notification.reference).one()
+    assert noti.billable_units == 1
+
+
 @freeze_time('2020-02-17 18:00:00')
 def test_get_key_and_size_of_letters_to_be_sent_to_print(notify_api, mocker, sample_letter_template):
     create_notification(