From e1d150a88237388f70e7c8b5db00d168504d4393 Mon Sep 17 00:00:00 2001 From: venusbb Date: Fri, 12 Jan 2018 15:10:42 +0000 Subject: [PATCH] Added process for dvla acknowledgement file Daily schedule task to check ack file against zip file lists if we haven't receive ack for a zip file, raise a 500 exception --- app/aws/s3.py | 16 +++++ app/celery/scheduled_tasks.py | 36 ++++++---- app/config.py | 2 +- .../notifications_letter_callback.py | 5 +- app/v2/errors.py | 17 +++++ requirements.txt | 2 +- tests/app/celery/test_scheduled_tasks.py | 50 +++++++++++++- tests/app/s3/test_s3.py | 66 +++++++++++++++++++ 8 files changed, 175 insertions(+), 19 deletions(-) create mode 100644 tests/app/s3/test_s3.py diff --git a/app/aws/s3.py b/app/aws/s3.py index 5ffb24068..6771921e7 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -107,3 +107,19 @@ def upload_letters_pdf(reference, crown, filedata): current_app.logger.info("Uploading letters PDF {} to {}".format( upload_file_name, current_app.config['LETTERS_PDF_BUCKET_NAME'])) + + +def get_list_of_files_by_suffix(bucket_name, subfolder='', suffix=''): + s3_client = client('s3', current_app.config['AWS_REGION']) + paginator = s3_client.get_paginator('list_objects_v2') + + page_iterator = paginator.paginate( + Bucket=bucket_name, + Prefix=subfolder + ) + + for page in page_iterator: + for obj in page['Contents']: + key = obj['Key'] + if key.endswith(suffix): + yield key diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index c6a36aca9..ca082657f 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -59,7 +59,7 @@ from app.celery.tasks import ( ) from app.config import QueueNames, TaskNames from app.utils import convert_utc_to_bst -from app.v2.errors import JobIncompleteError +from app.v2.errors import JobIncompleteError, NoAckFileReceived from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.celery.service_callback_tasks import send_delivery_status_to_service @@ -451,18 +451,30 @@ def daily_stats_template_usage_by_month(): @notify_celery.task(name='raise-alert-if-no-letter-ack-file') @statsd(namespace="tasks") -def raise_alert_if_no_letter_ack_file(): - """ - Get all files sent "today" - list_of_zip_files => get file names s3.get_s3_bucket_objects() look in the folder with todays date +def letter_raise_alert_if_no_ack_file_for_zip(): + # get a list of today's zip files + zip_file_list = [] + for key in s3.get_list_of_files_by_suffix(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], + subfolder=datetime.utcnow().strftime('%Y-%m-%d'), suffix='.ZIP'): + zip_file_list.append(key) - list_of_ack_files => Get all files sent today with name containing ack.txt from a diff bucket + # get acknowledgement file + ack_file_list = [] + for key in s3.get_list_of_files_by_suffix(bucket_name=current_app.config['DVLA_RESPONSE_BUCKET_NAME'], + subfolder='root/dispatch', suffix='.ACK.txt'): + ack_file_list.append(key) - For filename in list_of_zip_files: - for ack_file in list_of_ack_files if name= file.strip("NOTIFY.", ".ZIP") - IF no ack_file - raise NoAckFileReceived(status=500, message="No ack file received for {filename}") + todaystr = datetime.utcnow().strftime('%Y%m%d') - """ + for key in ack_file_list: + if todaystr in key: + content = s3.get_s3_file(current_app.config['DVLA_RESPONSE_BUCKET_NAME'], key) - pass \ No newline at end of file + for zip_file in content.split('\n'): # each line + s = zip_file.split('|') + for zf in zip_file_list: + if s[0] in zf: + zip_file_list.remove(zf) + + if zip_file_list: + raise NoAckFileReceived(message=zip_file_list) diff --git a/app/config.py b/app/config.py index 25a2cd80a..63beae32b 100644 --- a/app/config.py +++ b/app/config.py @@ -251,7 +251,7 @@ class Config(object): }, 'raise-alert-if-no-letter-ack-file': { 'task': 'raise-alert-if-no-letter-ack-file', - 'schedule': crontab(hour=19, minute=00), + 'schedule': crontab(hour=23, minute=00), 'options': {'queue': QueueNames.PERIODIC} }, 'run-letter-api-notifications': { diff --git a/app/notifications/notifications_letter_callback.py b/app/notifications/notifications_letter_callback.py index 06d24a40f..eac703186 100644 --- a/app/notifications/notifications_letter_callback.py +++ b/app/notifications/notifications_letter_callback.py @@ -53,9 +53,8 @@ def process_letter_response(): message = json.loads(req_json['Message']) filename = message['Records'][0]['s3']['object']['key'] current_app.logger.info('Received file from DVLA: {}'.format(filename)) - # IF file name contains .rs.txt THEN continue ELSE log message with file name. and return - current_app.logger.info('DVLA callback: Calling task to update letter notifications') - update_letter_notifications_statuses.apply_async([filename], queue=QueueNames.NOTIFY) + if 'rs.txt' in filename.lower(): + update_letter_notifications_statuses.apply_async([filename], queue=QueueNames.NOTIFY) return jsonify( result="success", message="DVLA callback succeeded" diff --git a/app/v2/errors.py b/app/v2/errors.py index a70d61d53..bba50b162 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -26,6 +26,23 @@ class JobIncompleteError(Exception): } +class NoAckFileReceived(Exception): + def __init__(self, message): + self.message = message + self.status_code = 500 + + def to_dict_v2(self): + return { + 'status_code': self.status_code, + "errors": [ + { + "error": 'NoAckFileReceived', + "message": str(self.message) + } + ] + } + + class TooManyRequestsError(InvalidRequest): status_code = 429 message_template = 'Exceeded send limits ({}) for today' diff --git a/requirements.txt b/requirements.txt index 83f2b5a9e..21a7b9ea1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,6 +25,6 @@ notifications-python-client==4.7.1 awscli==1.14.16 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@23.4.0#egg=notifications-utils==23.4.0 +git+https://github.com/alphagov/notifications-utils.git@23.4.1#egg=notifications-utils==23.4.1 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 83f6a6696..5054afd82 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -34,7 +34,8 @@ from app.celery.scheduled_tasks import ( switch_current_sms_provider_on_slow_delivery, timeout_job_statistics, timeout_notifications, - daily_stats_template_usage_by_month + daily_stats_template_usage_by_month, + letter_raise_alert_if_no_ack_file_for_zip ) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.config import QueueNames, TaskNames @@ -60,7 +61,7 @@ from app.models import ( SMS_TYPE ) from app.utils import get_london_midnight_in_utc -from app.v2.errors import JobIncompleteError +from app.v2.errors import JobIncompleteError, NoAckFileReceived from tests.app.db import create_notification, create_service, create_template, create_job, create_rate from tests.app.conftest import ( @@ -1026,3 +1027,48 @@ def test_dao_fetch_monthly_historical_stats_by_template_null_template_id_not_cou ).all() assert len(result) == 1 + + +def mock_s3_get_list_match(bucket_name, subfolder='', suffix=''): + + if subfolder == '2018-01-11': + return ['NOTIFY.20180111175007.ZIP', 'NOTIFY.20180111175008.ZIP'] + print(suffix) + if subfolder == 'root/dispatch': + return ['root/dispatch/NOTIFY.20180111175733.ACK.txt'] + + +def mock_s3_get_list_diff(bucket_name, subfolder='', suffix=''): + + if subfolder == '2018-01-11': + return ['NOTIFY.20180111175007.ZIP', 'NOTIFY.20180111175008.ZIP', 'NOTIFY.20180111175009.ZIP'] + print(suffix) + if subfolder == 'root/dispatch': + return ['root/dispatch/NOTIFY.20180111175733.ACK.txt'] + + +@freeze_time('2018-01-11T23:00:00') +def test_letter_not_raise_alert_if_ack_files_match_zip_list(mocker, notify_db): + mock_file_list = mocker.patch("app.aws.s3.get_list_of_files_by_suffix", side_effect=mock_s3_get_list_match) + mock_get_file = mocker.patch("app.aws.s3.get_s3_file", + return_value='NOTIFY.20180111175007.ZIP|20180111175733\n' + 'NOTIFY.20180111175008.ZIP|20180111175734') + + letter_raise_alert_if_no_ack_file_for_zip() + + assert mock_file_list.call_count == 2 + assert mock_get_file.call_count == 1 + + +@freeze_time('2018-01-11T23:00:00') +def test_letter_not_raise_alert_if_ack_files_not_match_zip_list(mocker, notify_db): + mock_file_list = mocker.patch("app.aws.s3.get_list_of_files_by_suffix", side_effect=mock_s3_get_list_diff) + mock_get_file = mocker.patch("app.aws.s3.get_s3_file", + return_value='NOTIFY.20180111175007.ZIP|20180111175733\n' + 'NOTIFY.20180111175008.ZIP|20180111175734') + with pytest.raises(expected_exception=NoAckFileReceived) as e: + letter_raise_alert_if_no_ack_file_for_zip() + + assert e.value.message == ['NOTIFY.20180111175009.ZIP'] + assert mock_file_list.call_count == 2 + assert mock_get_file.call_count == 1 diff --git a/tests/app/s3/test_s3.py b/tests/app/s3/test_s3.py new file mode 100644 index 000000000..9f36a2cdc --- /dev/null +++ b/tests/app/s3/test_s3.py @@ -0,0 +1,66 @@ +from app.aws.s3 import get_list_of_files_by_suffix, get_s3_file + + +zip_bucket_name = 'development-letters-pdf' +zip_sub_folder = '2018-01-11' +zip_file_name = '2018-01-11/NOTIFY.20180111175007.ZIP' +ack_bucket_name = 'development-letters-pdf' +ack_subfolder = 'root/dispatch' +ack_file_name = 'root/dispatch/NOTIFY.20180111175733.ACK.txt' + +# Tests for boto3 and s3, can only perform locally against the Tools aws account and have permissions to access S3. +# The tests are based on the above folders and files already uploaded to S3 Tools aws account (If these are removed or +# renamed, the tests won't pass. + + +def test_get_zip_files(): + zip_file_list = [] + for key in get_list_of_files_by_suffix(bucket_name=zip_bucket_name, subfolder=zip_sub_folder, suffix='.ZIP'): + print('File: ' + key) + zip_file_list.append(key) + assert zip_file_name in zip_file_list + + +def test_get_ack_files(): + ack_file_list = [] + for key in get_list_of_files_by_suffix(bucket_name=ack_bucket_name, subfolder=ack_subfolder, suffix='.ACK.txt'): + print('File: ' + key) + ack_file_list.append(key) + assert ack_file_name in ack_file_list + + +def test_get_file_content(): + ack_file_list = [] + for key in get_list_of_files_by_suffix(bucket_name=ack_bucket_name, subfolder=ack_subfolder, suffix='.ACK.txt'): + ack_file_list.append(key) + assert ack_file_name in key + + todaystr = '20180111' + for key in ack_file_list: + if todaystr in key: + content = get_s3_file(ack_bucket_name, key) + print(content) + + +def test_letter_ack_file_strip_correctly(): + # Test ack files are stripped correctly. In the acknowledgement file, there should be 2 zip files, + # 'NOTIFY.20180111175007.ZIP','NOTIFY.20180111175008.ZIP'. + zip_file_list = ['NOTIFY.20180111175007.ZIP', 'NOTIFY.20180111175008.ZIP', 'NOTIFY.20180111175009.ZIP'] + # get acknowledgement file + ack_file_list = [] + for key in get_list_of_files_by_suffix(bucket_name=ack_bucket_name, subfolder=ack_subfolder, suffix='.ACK.txt'): + ack_file_list.append(key) + + for key in ack_file_list: + if '20180111' in key: + content = get_s3_file(ack_bucket_name, key) + print(content) + for zip_file in content.split(): # iterate each line + s = zip_file.split('|') + print(s[0]) + for zf in zip_file_list: + if s[0] in zf: + zip_file_list.remove(zf) + + print('zip_file_list: ' + str(zip_file_list)) + assert zip_file_list == ['NOTIFY.20180111175009.ZIP']