Handle both new and old response type from template preview's

sanitise endpoint

Fix tests so they accept new response handling

app/celery/letters_pdf_tasks.py

@@ -1,5 +1,6 @@
 import io
 import math
+import base64
 from datetime import datetime
 from uuid import UUID
 from hashlib import sha512
@@ -48,6 +49,7 @@ from app.models import (
     NOTIFICATION_VIRUS_SCAN_FAILED,
 )
 from app.cronitor import cronitor
+from json import JSONDecodeError
 
 
 @notify_celery.task(bind=True, name="create-letters-pdf", max_retries=15, default_retry_delay=300)
@@ -213,8 +215,14 @@ def process_virus_scan_passed(self, filename):
         _move_invalid_letter_and_update_status(notification, filename, scan_pdf_object)
         return
 
-    new_pdf = _sanitise_precompiled_pdf(self, notification, old_pdf)
+    sanitise_response = _sanitise_precompiled_pdf(self, notification, old_pdf)
-
+    if not sanitise_response:
+        new_pdf = None
+    else:
+        try:
+            new_pdf = base64.b64decode(sanitise_response.json()["file"].encode())
+        except JSONDecodeError:
+            new_pdf = sanitise_response.content
     # TODO: Remove this once CYSP update their template to not cross over the margins
     if notification.service_id == UUID('fe44178f-3b45-4625-9f85-2264a36dd9ec'):  # CYSP
         # Check your state pension submit letters with good addresses and notify tags, so just use their supplied pdf
@@ -291,7 +299,7 @@ def _upload_pdf_to_test_or_live_pdf_bucket(pdf_data, filename, is_test_letter):
 
 def _sanitise_precompiled_pdf(self, notification, precompiled_pdf):
     try:
-        resp = requests_post(
+        response = requests_post(
             '{}/precompiled/sanitise'.format(
                 current_app.config['TEMPLATE_PREVIEW_API_HOST']
             ),
@@ -300,12 +308,16 @@ def _sanitise_precompiled_pdf(self, notification, precompiled_pdf):
                      'Service-ID': str(notification.service_id),
                      'Notification-ID': str(notification.id)}
         )
-        resp.raise_for_status()
+        response.raise_for_status()
-        return resp.content
+        return response
     except RequestException as ex:
         if ex.response is not None and ex.response.status_code == 400:
+            message = "sanitise_precompiled_pdf validation error for notification: {}. ".format(notification.id)
+            if "message" in response.json():
+                message += response.json()["message"]
+
             current_app.logger.info(
-                "sanitise_precompiled_pdf validation error for notification: {}".format(notification.id)
+                message
             )
             return None
 

tests/app/celery/test_letters_pdf_tasks.py

@@ -1,5 +1,6 @@
 from unittest.mock import Mock, call, ANY
 
+import base64
 import boto3
 from PyPDF2.utils import PdfReadError
 from moto import mock_s3
@@ -411,30 +412,41 @@ def test_process_letter_task_check_virus_scan_passed(
     conn.create_bucket(Bucket=target_bucket_name)
 
     s3 = boto3.client('s3', region_name='eu-west-1')
-    s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'pdf_content')
+    s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'old_pdf')
 
     mock_get_page_count = mocker.patch('app.celery.letters_pdf_tasks._get_page_count', return_value=1)
     mock_s3upload = mocker.patch('app.celery.letters_pdf_tasks.s3upload')
-    mock_sanitise = mocker.patch('app.celery.letters_pdf_tasks._sanitise_precompiled_pdf', return_value=b'pdf_content')
+    endpoint = 'http://localhost:9999/precompiled/sanitise'
-
+    with requests_mock.mock() as rmock:
+        rmock.request(
+            "POST",
+            endpoint,
+            json={
+                "file": base64.b64encode(b"new_pdf").decode("utf-8"),
+                "validation_passed": True,
+                "errors": {
+                    "content_outside_of_printable_area": [],
+                    "document_not_a4_size_portrait_orientation": [],
+                }
+            },
+            status_code=200
+        )
         process_virus_scan_passed(filename)
 
     assert letter_notification.status == noti_status
     assert letter_notification.billable_units == 1
-    mock_sanitise.assert_called_once_with(
+    assert rmock.called
-        ANY,
+    assert rmock.request_history[0].url == endpoint
-        letter_notification,
+
-        b'pdf_content'
-    )
     mock_s3upload.assert_called_once_with(
         bucket_name=target_bucket_name,
-        filedata=b'pdf_content',
+        filedata=b'new_pdf',
         file_location=destination_folder + filename,
         region='eu-west-1',
     )
     mock_get_page_count.assert_called_once_with(
         letter_notification,
-        b'pdf_content'
+        b'old_pdf'
     )
 
 
@@ -540,7 +552,6 @@ def test_process_virus_scan_passed_logs_error_and_sets_tech_failure_if_s3_error_
     s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'pdf_content')
 
     mocker.patch('app.celery.letters_pdf_tasks._get_page_count', return_value=1)
-    mocker.patch('app.celery.letters_pdf_tasks._sanitise_precompiled_pdf', return_value=b'pdf_content')
 
     error_response = {
         'Error': {
@@ -552,6 +563,21 @@ def test_process_virus_scan_passed_logs_error_and_sets_tech_failure_if_s3_error_
     mocker.patch('app.celery.letters_pdf_tasks._upload_pdf_to_test_or_live_pdf_bucket',
                  side_effect=ClientError(error_response, 'operation_name'))
 
+    endpoint = 'http://localhost:9999/precompiled/sanitise'
+    with requests_mock.mock() as rmock:
+        rmock.request(
+            "POST",
+            endpoint,
+            json={
+                "file": base64.b64encode(b"new_pdf").decode("utf-8"),
+                "validation_passed": True,
+                "errors": {
+                    "content_outside_of_printable_area": [],
+                    "document_not_a4_size_portrait_orientation": [],
+                }
+            },
+            status_code=200
+        )
         process_virus_scan_passed(filename)
 
     assert sample_letter_notification.status == NOTIFICATION_TECHNICAL_FAILURE
@@ -631,23 +657,54 @@ def test_replay_letters_in_error_for_one_file(notify_api, mocker):
 
 def test_sanitise_precompiled_pdf_returns_data_from_template_preview(rmock, sample_letter_notification):
     sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK
-    rmock.post('http://localhost:9999/precompiled/sanitise', content=b'new_pdf', status_code=200)
+    endpoint = 'http://localhost:9999/precompiled/sanitise'
+    with requests_mock.mock() as rmock:
+        rmock.request(
+            "POST",
+            endpoint,
+            json={
+                "file": base64.b64encode(b"new_pdf").decode("utf-8"),
+                "validation_passed": True,
+                "errors": {
+                    "content_outside_of_printable_area": [],
+                    "document_not_a4_size_portrait_orientation": [],
+                }
+            },
+            status_code=200
+        )
         mock_celery = Mock(**{'retry.side_effect': Retry})
+        response = _sanitise_precompiled_pdf(mock_celery, sample_letter_notification, b'old_pdf')
+        assert rmock.called
+        assert rmock.request_history[0].url == endpoint
 
-    res = _sanitise_precompiled_pdf(mock_celery, sample_letter_notification, b'old_pdf')
+    assert base64.b64decode(response.json()["file"].encode()) == b"new_pdf"
-
-    assert res == b'new_pdf'
     assert rmock.last_request.text == 'old_pdf'
 
 
 def test_sanitise_precompiled_pdf_returns_none_on_validation_error(rmock, sample_letter_notification):
     sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK
-    rmock.post('http://localhost:9999/precompiled/sanitise', content=b'new_pdf', status_code=400)
+
+    endpoint = 'http://localhost:9999/precompiled/sanitise'
+    with requests_mock.mock() as rmock:
+        rmock.request(
+            "POST",
+            endpoint,
+            json={
+                "file": base64.b64encode(b"nyan").decode("utf-8"),
+                "validation_passed": False,
+                "errors": {
+                    "content_outside_of_printable_area": [1],
+                    "document_not_a4_size_portrait_orientation": [],
+                }
+            },
+            status_code=400
+        )
         mock_celery = Mock(**{'retry.side_effect': Retry})
+        response = _sanitise_precompiled_pdf(mock_celery, sample_letter_notification, b'old_pdf')
+        assert rmock.called
+        assert rmock.request_history[0].url == endpoint
 
-    res = _sanitise_precompiled_pdf(mock_celery, sample_letter_notification, b'old_pdf')
+    assert response is None
-
-    assert res is None
 
 
 def test_sanitise_precompiled_pdf_passes_the_service_id_and_notification_id_to_template_preview(