diff --git a/app/main/s3_client.py b/app/main/s3_client.py deleted file mode 100644 index 5fc79c6f2..000000000 --- a/app/main/s3_client.py +++ /dev/null @@ -1,144 +0,0 @@ -import uuid - -import botocore -from boto3 import resource -from flask import current_app -from notifications_utils.s3 import s3upload as utils_s3upload - -FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' -TEMP_TAG = 'temp-{user_id}_' -LOGO_LOCATION_STRUCTURE = '{temp}{unique_id}-{filename}' - - -def get_s3_object(bucket_name, filename): - s3 = resource('s3') - return s3.Object(bucket_name, filename) - - -def get_csv_location(service_id, upload_id): - return ( - current_app.config['CSV_UPLOAD_BUCKET_NAME'], - FILE_LOCATION_STRUCTURE.format(service_id, upload_id), - ) - - -def get_csv_upload(service_id, upload_id): - return get_s3_object(*get_csv_location(service_id, upload_id)) - - -def delete_s3_object(filename): - bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - get_s3_object(bucket_name, filename).delete() - - -def persist_logo(old_name, new_name): - if old_name == new_name: - return - bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - get_s3_object(bucket_name, new_name).copy_from( - CopySource='{}/{}'.format(bucket_name, old_name)) - delete_s3_object(old_name) - - -def get_s3_objects_filter_by_prefix(prefix): - bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - s3 = resource('s3') - return s3.Bucket(bucket_name).objects.filter(Prefix=prefix) - - -def get_temp_truncated_filename(filename, user_id): - return filename[len(TEMP_TAG.format(user_id=user_id)):] - - -def s3upload(service_id, filedata, region): - upload_id = str(uuid.uuid4()) - bucket_name, file_location = get_csv_location(service_id, upload_id) - utils_s3upload( - filedata=filedata['data'], - region=region, - bucket_name=bucket_name, - file_location=file_location, - ) - return upload_id - - -def s3download(service_id, upload_id): - contents = '' - try: - key = get_csv_upload(service_id, upload_id) - contents = key.get()['Body'].read().decode('utf-8') - except botocore.exceptions.ClientError as e: - current_app.logger.error("Unable to download s3 file {}".format( - FILE_LOCATION_STRUCTURE.format(service_id, upload_id))) - raise e - return contents - - -def get_mou(organisation_is_crown): - bucket = current_app.config['MOU_BUCKET_NAME'] - filename = 'crown.pdf' if organisation_is_crown else 'non-crown.pdf' - attachment_filename = 'GOV.UK Notify data sharing and financial agreement{}.pdf'.format( - '' if organisation_is_crown else ' (non-crown)' - ) - try: - key = get_s3_object(bucket, filename) - return { - 'filename_or_fp': key.get()['Body'], - 'attachment_filename': attachment_filename, - 'as_attachment': True, - } - except botocore.exceptions.ClientError as exception: - current_app.logger.error("Unable to download s3 file {}/{}".format( - bucket, filename - )) - raise exception - - -def upload_logo(filename, filedata, region, user_id): - upload_file_name = LOGO_LOCATION_STRUCTURE.format( - temp=TEMP_TAG.format(user_id=user_id), - unique_id=str(uuid.uuid4()), - filename=filename - ) - bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - utils_s3upload( - filedata=filedata, - region=region, - bucket_name=bucket_name, - file_location=upload_file_name, - content_type='image/png' - ) - - return upload_file_name - - -def permanent_logo_name(filename, user_id): - if filename.startswith(TEMP_TAG.format(user_id=user_id)): - return get_temp_truncated_filename(filename=filename, user_id=user_id) - else: - return filename - - -def delete_temp_files_created_by(user_id): - for obj in get_s3_objects_filter_by_prefix(TEMP_TAG.format(user_id=user_id)): - delete_s3_object(obj.key) - - -def delete_temp_file(filename): - if not filename.startswith(TEMP_TAG[:5]): - raise ValueError('Not a temp file: {}'.format(filename)) - - delete_s3_object(filename) - - -def set_metadata_on_csv_upload(service_id, upload_id, **kwargs): - get_csv_upload( - service_id, upload_id - ).copy_from( - CopySource='{}/{}'.format(*get_csv_location(service_id, upload_id)), - ServerSideEncryption='AES256', - Metadata={ - key: str(value) for key, value in kwargs.items() - }, - MetadataDirective='REPLACE', - ) diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index b3e41dbb5..3862be01c 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -2,8 +2,8 @@ from flask import abort, render_template, request, send_file, url_for from flask_login import login_required from app.main import main -from app.main.s3_client import get_mou from app.main.views.sub_navigation_dictionaries import features_nav +from app.s3_client.s3_mou_client import get_mou from app.utils import AgreementInfo diff --git a/app/main/views/email_branding.py b/app/main/views/email_branding.py index 5a5b3f9a4..6b754df11 100644 --- a/app/main/views/email_branding.py +++ b/app/main/views/email_branding.py @@ -4,7 +4,7 @@ from flask_login import login_required from app import email_branding_client from app.main import main from app.main.forms import SearchTemplatesForm, ServiceUpdateEmailBranding -from app.main.s3_client import ( +from app.s3_client.s3_logo_client import ( TEMP_TAG, delete_temp_file, delete_temp_files_created_by, diff --git a/app/main/views/send.py b/app/main/views/send.py index c85280185..e61dd8935 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -42,7 +42,11 @@ from app.main.forms import ( SetSenderForm, get_placeholder_form_instance, ) -from app.main.s3_client import s3download, s3upload, set_metadata_on_csv_upload +from app.s3_client.s3_csv_client import ( + s3download, + s3upload, + set_metadata_on_csv_upload, +) from app.template_previews import TemplatePreview, get_page_count_for_letter from app.utils import ( Spreadsheet, diff --git a/app/s3_client/__init__.py b/app/s3_client/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/s3_client/s3_csv_client.py b/app/s3_client/s3_csv_client.py new file mode 100644 index 000000000..f9d0b5b9f --- /dev/null +++ b/app/s3_client/s3_csv_client.py @@ -0,0 +1,57 @@ +import uuid + +import botocore +from flask import current_app +from notifications_utils.s3 import s3upload as utils_s3upload + +from app.s3_client.s3_logo_client import get_s3_object + +FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' + + +def get_csv_location(service_id, upload_id): + return ( + current_app.config['CSV_UPLOAD_BUCKET_NAME'], + FILE_LOCATION_STRUCTURE.format(service_id, upload_id), + ) + + +def get_csv_upload(service_id, upload_id): + return get_s3_object(*get_csv_location(service_id, upload_id)) + + +def s3upload(service_id, filedata, region): + upload_id = str(uuid.uuid4()) + bucket_name, file_location = get_csv_location(service_id, upload_id) + utils_s3upload( + filedata=filedata['data'], + region=region, + bucket_name=bucket_name, + file_location=file_location, + ) + return upload_id + + +def s3download(service_id, upload_id): + contents = '' + try: + key = get_csv_upload(service_id, upload_id) + contents = key.get()['Body'].read().decode('utf-8') + except botocore.exceptions.ClientError as e: + current_app.logger.error("Unable to download s3 file {}".format( + FILE_LOCATION_STRUCTURE.format(service_id, upload_id))) + raise e + return contents + + +def set_metadata_on_csv_upload(service_id, upload_id, **kwargs): + get_csv_upload( + service_id, upload_id + ).copy_from( + CopySource='{}/{}'.format(*get_csv_location(service_id, upload_id)), + ServerSideEncryption='AES256', + Metadata={ + key: str(value) for key, value in kwargs.items() + }, + MetadataDirective='REPLACE', + ) diff --git a/app/s3_client/s3_logo_client.py b/app/s3_client/s3_logo_client.py new file mode 100644 index 000000000..a9ca81b97 --- /dev/null +++ b/app/s3_client/s3_logo_client.py @@ -0,0 +1,74 @@ +import uuid + +from boto3 import resource +from flask import current_app +from notifications_utils.s3 import s3upload as utils_s3upload + +TEMP_TAG = 'temp-{user_id}_' +LOGO_LOCATION_STRUCTURE = '{temp}{unique_id}-{filename}' + + +def get_s3_object(bucket_name, filename): + s3 = resource('s3') + return s3.Object(bucket_name, filename) + + +def delete_s3_object(filename): + bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] + get_s3_object(bucket_name, filename).delete() + + +def persist_logo(old_name, new_name): + if old_name == new_name: + return + bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] + get_s3_object(bucket_name, new_name).copy_from( + CopySource='{}/{}'.format(bucket_name, old_name)) + delete_s3_object(old_name) + + +def get_s3_objects_filter_by_prefix(prefix): + bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] + s3 = resource('s3') + return s3.Bucket(bucket_name).objects.filter(Prefix=prefix) + + +def get_temp_truncated_filename(filename, user_id): + return filename[len(TEMP_TAG.format(user_id=user_id)):] + + +def upload_logo(filename, filedata, region, user_id): + upload_file_name = LOGO_LOCATION_STRUCTURE.format( + temp=TEMP_TAG.format(user_id=user_id), + unique_id=str(uuid.uuid4()), + filename=filename + ) + bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] + utils_s3upload( + filedata=filedata, + region=region, + bucket_name=bucket_name, + file_location=upload_file_name, + content_type='image/png' + ) + + return upload_file_name + + +def permanent_logo_name(filename, user_id): + if filename.startswith(TEMP_TAG.format(user_id=user_id)): + return get_temp_truncated_filename(filename=filename, user_id=user_id) + else: + return filename + + +def delete_temp_files_created_by(user_id): + for obj in get_s3_objects_filter_by_prefix(TEMP_TAG.format(user_id=user_id)): + delete_s3_object(obj.key) + + +def delete_temp_file(filename): + if not filename.startswith(TEMP_TAG[:5]): + raise ValueError('Not a temp file: {}'.format(filename)) + + delete_s3_object(filename) diff --git a/app/s3_client/s3_mou_client.py b/app/s3_client/s3_mou_client.py new file mode 100644 index 000000000..d86e7b2cc --- /dev/null +++ b/app/s3_client/s3_mou_client.py @@ -0,0 +1,24 @@ +import botocore +from flask import current_app + +from app.s3_client.s3_logo_client import get_s3_object + + +def get_mou(organisation_is_crown): + bucket = current_app.config['MOU_BUCKET_NAME'] + filename = 'crown.pdf' if organisation_is_crown else 'non-crown.pdf' + attachment_filename = 'GOV.UK Notify data sharing and financial agreement{}.pdf'.format( + '' if organisation_is_crown else ' (non-crown)' + ) + try: + key = get_s3_object(bucket, filename) + return { + 'filename_or_fp': key.get()['Body'], + 'attachment_filename': attachment_filename, + 'as_attachment': True, + } + except botocore.exceptions.ClientError as exception: + current_app.logger.error("Unable to download s3 file {}/{}".format( + bucket, filename + )) + raise exception diff --git a/app/utils.py b/app/utils.py index 0f7ee1134..c7ed47090 100644 --- a/app/utils.py +++ b/app/utils.py @@ -126,7 +126,7 @@ def get_errors_for_csv(recipients, template_type): def generate_notifications_csv(**kwargs): from app import notification_api_client - from app.main.s3_client import s3download + from app.s3_client.s3_csv_client import s3download if 'page' not in kwargs: kwargs['page'] = 1 diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index 02bc6b34e..8da0fd3e2 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -78,7 +78,7 @@ def test_downloading_agreement( expected_file_served, ): mock_get_s3_object = mocker.patch( - 'app.main.s3_client.get_s3_object', + 'app.s3_client.s3_mou_client.get_s3_object', return_value=_MockS3Object(b'foo') ) user = active_user_with_permissions(fake_uuid) @@ -100,7 +100,7 @@ def test_agreement_cant_be_downloaded_unknown_crown_status( fake_uuid, ): mock_get_s3_object = mocker.patch( - 'app.main.s3_client.get_s3_object', + 'app.s3_client.s3_mou_client.get_s3_object', return_value=_MockS3Object() ) user = active_user_with_permissions(fake_uuid) @@ -116,7 +116,7 @@ def test_agreement_requires_login( mocker, ): mock_get_s3_object = mocker.patch( - 'app.main.s3_client.get_s3_object', + 'app.s3_client.s3_mou_client.get_s3_object', return_value=_MockS3Object() ) response = client.get(url_for('main.download_agreement')) @@ -142,7 +142,7 @@ def test_show_public_agreement_page( expected_status, ): mocker.patch( - 'app.main.s3_client.get_s3_object', + 'app.s3_client.s3_mou_client.get_s3_object', return_value=_MockS3Object() ) response = client.get(url_for( diff --git a/tests/app/main/views/test_email_branding.py b/tests/app/main/views/test_email_branding.py index 6b8d1f318..3dd168537 100644 --- a/tests/app/main/views/test_email_branding.py +++ b/tests/app/main/views/test_email_branding.py @@ -6,7 +6,7 @@ from bs4 import BeautifulSoup from flask import url_for from notifications_python_client.errors import HTTPError -from app.main.s3_client import LOGO_LOCATION_STRUCTURE, TEMP_TAG +from app.s3_client.s3_logo_client import LOGO_LOCATION_STRUCTURE, TEMP_TAG from tests.conftest import ( mock_get_email_branding, normalize_spaces, diff --git a/tests/app/s3_client/__init__.py b/tests/app/s3_client/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/s3_client/test_s3_csv_client.py b/tests/app/s3_client/test_s3_csv_client.py new file mode 100644 index 000000000..f4014065f --- /dev/null +++ b/tests/app/s3_client/test_s3_csv_client.py @@ -0,0 +1,21 @@ +from unittest.mock import Mock + +from app.s3_client.s3_csv_client import set_metadata_on_csv_upload + + +def test_sets_metadata(client, mocker): + mocked_s3_object = Mock() + mocked_get_s3_object = mocker.patch( + 'app.s3_client.s3_csv_client.get_csv_upload', + return_value=mocked_s3_object, + ) + + set_metadata_on_csv_upload('1234', '5678', foo='bar', baz=True) + + mocked_get_s3_object.assert_called_once_with('1234', '5678') + mocked_s3_object.copy_from.assert_called_once_with( + CopySource='test-notifications-csv-upload/service-1234-notify/5678.csv', + Metadata={'baz': 'True', 'foo': 'bar'}, + MetadataDirective='REPLACE', + ServerSideEncryption='AES256', + ) diff --git a/tests/app/main/test_s3_client.py b/tests/app/s3_client/test_s3_logo_client.py similarity index 67% rename from tests/app/main/test_s3_client.py rename to tests/app/s3_client/test_s3_logo_client.py index 20c6d80a3..332598f78 100644 --- a/tests/app/main/test_s3_client.py +++ b/tests/app/s3_client/test_s3_logo_client.py @@ -1,16 +1,15 @@ from collections import namedtuple -from unittest.mock import Mock, call +from unittest.mock import call import pytest -from app.main.s3_client import ( +from app.s3_client.s3_logo_client import ( LOGO_LOCATION_STRUCTURE, TEMP_TAG, delete_temp_file, delete_temp_files_created_by, permanent_logo_name, persist_logo, - set_metadata_on_csv_upload, upload_logo, ) @@ -30,7 +29,7 @@ def upload_filename(fake_uuid): def test_upload_logo_calls_correct_args(client, mocker, fake_uuid, upload_filename): mocker.patch('uuid.uuid4', return_value=upload_id) mocker.patch.dict('flask.current_app.config', {'LOGO_UPLOAD_BUCKET_NAME': bucket}) - mocked_s3_upload = mocker.patch('app.main.s3_client.utils_s3upload') + mocked_s3_upload = mocker.patch('app.s3_client.s3_logo_client.utils_s3upload') upload_logo(filename=filename, user_id=fake_uuid, filedata=data, region=region) @@ -44,8 +43,8 @@ def test_upload_logo_calls_correct_args(client, mocker, fake_uuid, upload_filena def test_persist_logo(client, mocker, fake_uuid, upload_filename): mocker.patch.dict('flask.current_app.config', {'LOGO_UPLOAD_BUCKET_NAME': bucket}) - mocked_get_s3_object = mocker.patch('app.main.s3_client.get_s3_object') - mocked_delete_s3_object = mocker.patch('app.main.s3_client.delete_s3_object') + mocked_get_s3_object = mocker.patch('app.s3_client.s3_logo_client.get_s3_object') + mocked_delete_s3_object = mocker.patch('app.s3_client.s3_logo_client.delete_s3_object') new_filename = permanent_logo_name(upload_filename, fake_uuid) @@ -59,8 +58,8 @@ def test_persist_logo_returns_if_not_temp(client, mocker, fake_uuid): filename = 'logo.png' persist_logo(filename, filename) - mocked_get_s3_object = mocker.patch('app.main.s3_client.get_s3_object') - mocked_delete_s3_object = mocker.patch('app.main.s3_client.delete_s3_object') + mocked_get_s3_object = mocker.patch('app.s3_client.s3_logo_client.get_s3_object') + mocked_delete_s3_object = mocker.patch('app.s3_client.s3_logo_client.delete_s3_object') mocked_get_s3_object.assert_not_called() mocked_delete_s3_object.assert_not_called() @@ -83,8 +82,8 @@ def test_delete_temp_files_created_by_user(client, mocker, fake_uuid): obj = namedtuple("obj", ["key"]) objs = [obj(key='test1'), obj(key='test2')] - mocker.patch('app.main.s3_client.get_s3_objects_filter_by_prefix', return_value=objs) - mocked_delete_s3_object = mocker.patch('app.main.s3_client.delete_s3_object') + mocker.patch('app.s3_client.s3_logo_client.get_s3_objects_filter_by_prefix', return_value=objs) + mocked_delete_s3_object = mocker.patch('app.s3_client.s3_logo_client.delete_s3_object') delete_temp_files_created_by(fake_uuid) @@ -94,7 +93,7 @@ def test_delete_temp_files_created_by_user(client, mocker, fake_uuid): def test_delete_single_temp_file(client, mocker, fake_uuid, upload_filename): - mocked_delete_s3_object = mocker.patch('app.main.s3_client.delete_s3_object') + mocked_delete_s3_object = mocker.patch('app.s3_client.s3_logo_client.delete_s3_object') delete_temp_file(upload_filename) @@ -103,28 +102,10 @@ def test_delete_single_temp_file(client, mocker, fake_uuid, upload_filename): def test_does_not_delete_non_temp_file(client, mocker, fake_uuid): filename = 'logo.png' - mocked_delete_s3_object = mocker.patch('app.main.s3_client.delete_s3_object') + mocked_delete_s3_object = mocker.patch('app.s3_client.s3_logo_client.delete_s3_object') with pytest.raises(ValueError) as error: delete_temp_file(filename) assert mocked_delete_s3_object.called_with_args(filename) assert str(error.value) == 'Not a temp file: {}'.format(filename) - - -def test_sets_metadata(client, mocker): - mocked_s3_object = Mock() - mocked_get_s3_object = mocker.patch( - 'app.main.s3_client.get_csv_upload', - return_value=mocked_s3_object, - ) - - set_metadata_on_csv_upload('1234', '5678', foo='bar', baz=True) - - mocked_get_s3_object.assert_called_once_with('1234', '5678') - mocked_s3_object.copy_from.assert_called_once_with( - CopySource='test-notifications-csv-upload/service-1234-notify/5678.csv', - Metadata={'baz': 'True', 'foo': 'bar'}, - MetadataDirective='REPLACE', - ServerSideEncryption='AES256', - ) diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index d65cafbb5..7bdfd4ebf 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -210,7 +210,7 @@ def test_generate_notifications_csv_returns_correct_csv_file( expected_1st_row, ): mocker.patch( - 'app.main.s3_client.s3download', + 'app.s3_client.s3_csv_client.s3download', return_value=original_file_contents, ) csv_content = generate_notifications_csv(service_id='1234', job_id=fake_uuid, template_type='sms') @@ -236,7 +236,7 @@ def test_generate_notifications_csv_calls_twice_if_next_link( ): mocker.patch( - 'app.main.s3_client.s3download', + 'app.s3_client.s3_csv_client.s3download', return_value=""" phone_number 07700900000