From 0cfed3f514e5dd5a1e3a6586a39108afaec04272 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 12 Jul 2018 16:53:10 +0100 Subject: [PATCH] create fake letter response files with variables timestamps when a test letter is created on dev or preview, we upload a file to the dvla ftp response bucket, to test that our integration with s3 works. s3 triggers an sns notification, which we pick up, and then we download the file and mark the letters it mentions as delivered. However, if two tests run at the same time, they'll create the same file on s3. One will just overwrite the next, and the first letter will never move into delivered - this was causing functional tests to intermittently fail. This commit makes the test letter task check if the file exists - if it does, it moves back one second and tries again. It tries this thirty times before giving up. --- app/aws/s3.py | 12 +++++++ app/celery/research_mode_tasks.py | 16 +++++++-- tests/app/celery/test_research_mode_tasks.py | 37 +++++++++++++++++++- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/app/aws/s3.py b/app/aws/s3.py index b23cca71d..a440745da 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -4,6 +4,7 @@ from flask import current_app import pytz from boto3 import client, resource +import botocore FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' @@ -18,6 +19,17 @@ def get_s3_object(bucket_name, file_location): return s3.Object(bucket_name, file_location) +def file_exists(bucket_name, file_location): + try: + # try and access metadata of object + get_s3_object(bucket_name, file_location).metadata + return True + except botocore.exceptions.ClientError as e: + if e.response['ResponseMetadata']['HTTPStatusCode'] == 404: + return False + raise + + def get_job_location(service_id, job_id): return ( current_app.config['CSV_UPLOAD_BUCKET_NAME'], diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index ac2e1e8dd..5fbe09b01 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, timedelta import json from flask import current_app @@ -7,6 +7,7 @@ from requests import request, RequestException, HTTPError from notifications_utils.s3 import s3upload from app import notify_celery +from app.aws.s3 import file_exists from app.models import SMS_TYPE from app.config import QueueNames from app.celery.process_ses_receipts_tasks import process_ses_results @@ -123,7 +124,18 @@ def firetext_callback(notification_id, to): def create_fake_letter_response_file(self, reference): now = datetime.utcnow() dvla_response_data = '{}|Sent|0|Sorted'.format(reference) - upload_file_name = 'NOTIFY-{}-RSP.TXT'.format(now.strftime('%Y%m%d%H%M%S')) + + # try and find a filename that hasn't been taken yet - going back in time 60 seconds + for i in range(30): + upload_file_name = 'NOTIFY-{}-RSP.TXT'.format((now - timedelta(seconds=i)).strftime('%Y%m%d%H%M%S')) + if not file_exists(current_app.config['DVLA_RESPONSE_BUCKET_NAME'], upload_file_name): + break + else: + raise ValueError( + 'cant create fake letter response file for {} - too many files for that time already exist on s3'.format( + reference + ) + ) s3upload( filedata=dvla_response_data, diff --git a/tests/app/celery/test_research_mode_tasks.py b/tests/app/celery/test_research_mode_tasks.py index 77656a62e..aacfd81d5 100644 --- a/tests/app/celery/test_research_mode_tasks.py +++ b/tests/app/celery/test_research_mode_tasks.py @@ -1,5 +1,5 @@ import uuid -from unittest.mock import ANY +from unittest.mock import ANY, call from flask import current_app, json from freezegun import freeze_time @@ -111,6 +111,7 @@ def test_failure_firetext_callback(phone_number): @freeze_time("2018-01-25 14:00:00") def test_create_fake_letter_response_file_uploads_response_file_s3( notify_api, mocker): + mocker.patch('app.celery.research_mode_tasks.file_exists', return_value=False) mock_s3upload = mocker.patch('app.celery.research_mode_tasks.s3upload') filename = 'NOTIFY-20180125140000-RSP.TXT' @@ -134,6 +135,7 @@ def test_create_fake_letter_response_file_uploads_response_file_s3( @freeze_time("2018-01-25 14:00:00") def test_create_fake_letter_response_file_calls_dvla_callback_on_development( notify_api, mocker): + mocker.patch('app.celery.research_mode_tasks.file_exists', return_value=False) mocker.patch('app.celery.research_mode_tasks.s3upload') filename = 'NOTIFY-20180125140000-RSP.TXT' @@ -159,6 +161,7 @@ def test_create_fake_letter_response_file_calls_dvla_callback_on_development( @freeze_time("2018-01-25 14:00:00") def test_create_fake_letter_response_file_does_not_call_dvla_callback_on_preview( notify_api, mocker): + mocker.patch('app.celery.research_mode_tasks.file_exists', return_value=False) mocker.patch('app.celery.research_mode_tasks.s3upload') with set_config_values(notify_api, { @@ -168,3 +171,35 @@ def test_create_fake_letter_response_file_does_not_call_dvla_callback_on_preview create_fake_letter_response_file('random-ref') assert request_mock.last_request is None + + +@freeze_time("2018-01-25 14:00:30") +def test_create_fake_letter_response_file_tries_to_create_files_with_other_filenames(notify_api, mocker): + mock_file_exists = mocker.patch('app.celery.research_mode_tasks.file_exists', side_effect=[True, True, False]) + mock_s3upload = mocker.patch('app.celery.research_mode_tasks.s3upload') + + create_fake_letter_response_file('random-ref') + + assert mock_file_exists.mock_calls == [ + call('test.notify.com-ftp', 'NOTIFY-20180125140030-RSP.TXT'), + call('test.notify.com-ftp', 'NOTIFY-20180125140029-RSP.TXT'), + call('test.notify.com-ftp', 'NOTIFY-20180125140028-RSP.TXT'), + ] + mock_s3upload.assert_called_once_with( + filedata=ANY, + region=ANY, + bucket_name=ANY, + file_location='NOTIFY-20180125140028-RSP.TXT' + ) + + +@freeze_time("2018-01-25 14:00:30") +def test_create_fake_letter_response_file_gives_up_after_thirty_times(notify_api, mocker): + mock_file_exists = mocker.patch('app.celery.research_mode_tasks.file_exists', return_value=True) + mock_s3upload = mocker.patch('app.celery.research_mode_tasks.s3upload') + + with pytest.raises(ValueError): + create_fake_letter_response_file('random-ref') + + assert len(mock_file_exists.mock_calls) == 30 + assert not mock_s3upload.called