diff --git a/app/aws/s3.py b/app/aws/s3.py index ed206b3a4..f5798314b 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -26,3 +26,10 @@ def remove_job_from_s3(service_id, job_id): file_location = FILE_LOCATION_STRUCTURE.format(service_id, job_id) obj = get_s3_object(bucket_name, file_location) return obj.delete() + + +def remove_transformed_dvla_file(job_id): + bucket_name = current_app.config['DVLA_UPLOAD_BUCKET_NAME'] + file_location = '{}-dvla-job.text'.format(job_id) + obj = get_s3_object(bucket_name, file_location) + return obj.delete() diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 636522f01..11e4de49d 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -24,6 +24,7 @@ from app.dao.provider_details_dao import ( dao_toggle_sms_provider ) from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago +from app.models import LETTER_TYPE from app.notifications.process_notifications import send_notification_to_queue from app.statsd_decorators import statsd from app.celery.tasks import process_job @@ -245,3 +246,12 @@ def delete_inbound_sms_older_than_seven_days(): except SQLAlchemyError as e: current_app.logger.exception("Failed to delete inbound sms notifications") raise + + +@notify_celery.task(name="remove_transformed_dvla_files") +@statsd(namespace="tasks") +def remove_transformed_dvla_files(): + jobs = dao_get_jobs_older_than_limited_by(job_types=[LETTER_TYPE]) + for job in jobs: + s3.remove_transformed_dvla_file(job.id) + current_app.logger.info("Transformed dvla file for job {} has been removed from s3.".format(job.id)) diff --git a/app/config.py b/app/config.py index 63d1f18c3..f470666a7 100644 --- a/app/config.py +++ b/app/config.py @@ -204,6 +204,11 @@ class Config(object): 'options': {'queue': QueueNames.PERIODIC}, 'kwargs': {'job_types': [LETTER_TYPE]} }, + 'remove_transformed_dvla_files': { + 'task': 'remove_transformed_dvla_files', + 'schedule': crontab(minute=40, hour=4), + 'options': {'queue': QueueNames.PERIODIC} + }, 'timeout-job-statistics': { 'task': 'timeout-job-statistics', 'schedule': crontab(minute=0, hour=5), diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index 08a0d3c5e..cd721f596 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -14,7 +14,7 @@ from app.v2.notifications.notification_schemas import get_notifications_request def get_notification_by_id(id): try: casted_id = uuid.UUID(id) - except ValueError or AttributeError: + except (ValueError, AttributeError): abort(404) notification = notifications_dao.get_notification_with_personalisation( authenticated_service.id, casted_id, key_type=None diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index 38c9966dd..fd30c16df 100644 --- a/tests/app/aws/test_s3.py +++ b/tests/app/aws/test_s3.py @@ -1,7 +1,11 @@ -from app.aws.s3 import get_s3_file +from unittest.mock import call + +from flask import current_app + +from app.aws.s3 import get_s3_file, remove_transformed_dvla_file -def test_get_s3_file_makes_correct_call(sample_service, sample_job, mocker): +def test_get_s3_file_makes_correct_call(notify_api, mocker): get_s3_mock = mocker.patch('app.aws.s3.get_s3_object') get_s3_file('foo-bucket', 'bar-file.txt') @@ -9,3 +13,15 @@ def test_get_s3_file_makes_correct_call(sample_service, sample_job, mocker): 'foo-bucket', 'bar-file.txt' ) + + +def test_remove_transformed_dvla_file_makes_correct_call(notify_api, mocker): + s3_mock = mocker.patch('app.aws.s3.get_s3_object') + fake_uuid = '5fbf9799-6b9b-4dbb-9a4e-74a939f3bb49' + + remove_transformed_dvla_file(fake_uuid) + + s3_mock.assert_has_calls([ + call(current_app.config['DVLA_UPLOAD_BUCKET_NAME'], '{}-dvla-job.text'.format(fake_uuid)), + call().delete() + ]) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index b3ff5d478..fcdd8ea93 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -1,10 +1,12 @@ -import pytest - from datetime import datetime, timedelta from functools import partial +from unittest.mock import call, patch, PropertyMock from flask import current_app + +import pytest from freezegun import freeze_time + from app.celery import scheduled_tasks from app.celery.scheduled_tasks import ( delete_email_notifications_older_than_seven_days, @@ -15,6 +17,7 @@ from app.celery.scheduled_tasks import ( delete_sms_notifications_older_than_seven_days, delete_verify_codes, remove_csv_files, + remove_transformed_dvla_files, run_scheduled_jobs, s3, send_daily_performance_platform_stats, @@ -41,7 +44,6 @@ from tests.app.conftest import ( sample_notification_history as create_notification_history, create_custom_template) from tests.conftest import set_config_values -from unittest.mock import call, patch, PropertyMock def _create_slow_delivery_notification(provider='mmg'): @@ -89,6 +91,8 @@ def test_should_have_decorated_tasks_functions(): 'switch_current_sms_provider_on_slow_delivery' assert delete_inbound_sms_older_than_seven_days.__wrapped__.__name__ == \ 'delete_inbound_sms_older_than_seven_days' + assert remove_transformed_dvla_files.__wrapped__.__name__ == \ + 'remove_transformed_dvla_files' def test_should_call_delete_sms_notifications_more_than_week_in_task(notify_api, mocker): @@ -488,3 +492,59 @@ def test_remove_csv_files_filters_by_type(mocker, sample_service): assert s3.remove_job_from_s3.call_args_list == [ call(job_to_delete.service_id, job_to_delete.id), ] + + +@freeze_time('2017-01-01 10:00:00') +def test_remove_dvla_transformed_files_removes_expected_files(mocker, sample_service): + mocker.patch('app.celery.scheduled_tasks.s3.remove_transformed_dvla_file') + + letter_template = create_template(service=sample_service, template_type=LETTER_TYPE) + + job = partial(create_job, template=letter_template) + + seven_days_ago = datetime.utcnow() - timedelta(days=7) + just_under_seven_days = seven_days_ago + timedelta(seconds=1) + just_over_seven_days = seven_days_ago - timedelta(seconds=1) + eight_days_ago = seven_days_ago - timedelta(days=1) + nine_days_ago = eight_days_ago - timedelta(days=1) + just_under_nine_days = nine_days_ago + timedelta(seconds=1) + just_over_nine_days = nine_days_ago - timedelta(seconds=1) + + job(created_at=seven_days_ago) + job(created_at=just_under_seven_days) + job_to_delete_1 = job(created_at=just_over_seven_days) + job_to_delete_2 = job(created_at=eight_days_ago) + job_to_delete_3 = job(created_at=nine_days_ago) + job_to_delete_4 = job(created_at=just_under_nine_days) + job(created_at=just_over_nine_days) + + remove_transformed_dvla_files() + + s3.remove_transformed_dvla_file.assert_has_calls([ + call(job_to_delete_1.id), + call(job_to_delete_2.id), + call(job_to_delete_3.id), + call(job_to_delete_4.id), + ], any_order=True) + + +def test_remove_dvla_transformed_files_does_not_remove_files(mocker, sample_service): + mocker.patch('app.celery.scheduled_tasks.s3.remove_transformed_dvla_file') + + letter_template = create_template(service=sample_service, template_type=LETTER_TYPE) + + job = partial(create_job, template=letter_template) + + yesterday = datetime.utcnow() - timedelta(days=1) + six_days_ago = datetime.utcnow() - timedelta(days=6) + seven_days_ago = six_days_ago - timedelta(days=1) + just_over_nine_days = seven_days_ago - timedelta(days=2, seconds=1) + + job(created_at=yesterday) + job(created_at=six_days_ago) + job(created_at=seven_days_ago) + job(created_at=just_over_nine_days) + + remove_transformed_dvla_files() + + s3.remove_transformed_dvla_file.assert_has_calls([])