darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-03-26 13:11:45 -04:00
Code Issues Packages Projects Releases Wiki Activity

Introduce abstraction for s3 metadata

Browse Source 
S3 metadata only supports ascii characters. Whenever we save data to it
we need to make sure we encode it to save it and then decode it to
display it again to users. This abstraction will act as the place for
that decoding to happen so the rest of the code in our views doesn't
need to care about the encoding abstraction.
This commit is contained in:
David McDonald
2020-05-18 15:15:54 +01:00
parent e79ea0e383
commit ba80d5b7cd
2 changed files with 30 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
app/s3_client/s3_letter_upload_client.py
View File

@@ -48,9 +48,20 @@ def get_letter_pdf_and_metadata(service_id, file_id):
s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get()
pdf = s3_object['Body'].read()
metadata = s3_object['Metadata']
return pdf, metadata
return pdf, LetterMetadata(s3_object['Metadata'])
class LetterMetadata:
KEYS_TO_DECODE = []
def __init__(self, metadata):
self._metadata = metadata
def get(self, key, default=None):
if key in self.KEYS_TO_DECODE:
return urllib.parse.unquote(self._metadata.get(key, default))
return self._metadata.get(key, default)
def get_letter_metadata(service_id, file_id):
@@ -58,4 +69,4 @@ def get_letter_metadata(service_id, file_id):
s3 = resource('s3')
s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get()
return s3_object['Metadata']
return LetterMetadata(s3_object['Metadata'])

31
tests/app/main/views/test_uploads.py
View File

@@ -10,6 +10,7 @@ from freezegun import freeze_time
from requests import RequestException
from app.main.views.uploads import format_recipient
from app.s3_client.s3_letter_upload_client import LetterMetadata
from app.utils import normalize_spaces
from tests.conftest import (
SERVICE_ONE_ID,
@@ -166,12 +167,12 @@ def test_post_upload_letter_redirects_for_valid_file(
)
)
mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3')
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
'filename': 'tests/test_pdf_files/one_page_pdf.pdf',
'page_count': '1',
'status': 'valid',
'recipient': 'The Queen'
})
}))
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template')
service_one['restricted'] = False
@@ -232,12 +233,12 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(
)
mocker.patch('app.main.views.uploads.upload_letter_to_s3')
mocker.patch('app.main.views.uploads.pdf_page_count', return_value=3)
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
'filename': 'tests/test_pdf_files/one_page_pdf.pdf',
'page_count': '3',
'status': 'valid',
'recipient': 'The Queen'
})
}))
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template)
service_one['restricted'] = False
@@ -351,9 +352,9 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request, fake_uuid)
}
mocker.patch('app.main.views.uploads.sanitise_letter', return_value=mock_sanitise_response)
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template')
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'invalid',
'message': 'content-outside-printable-area', 'invalid_pages': '[1]'})
'message': 'content-outside-printable-area', 'invalid_pages': '[1]'}))
with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file:
file_contents = file.read()
@@ -395,9 +396,9 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client
mock_sanitise_response.json = lambda: {"message": "template preview error", "recipient_address": "The Queen"}
mocker.patch('app.main.views.uploads.sanitise_letter', return_value=mock_sanitise_response)
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template)
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'invalid',
'message': 'template-preview-error'})
'message': 'template-preview-error'}))
with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file:
page = client_request.post(
@@ -455,10 +456,10 @@ def test_uploaded_letter_preview(
):
mocker.patch('app.main.views.uploads.service_api_client')
recipient = 'Bugs Bunny\n123 Big Hole\rLooney Town'
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid',
'recipient': urllib.parse.quote(recipient)
})
}))
service_one['restricted'] = False
client_request.login(active_user_with_permissions, service=service_one)
@@ -485,8 +486,8 @@ def test_uploaded_letter_preview_does_not_show_send_button_if_service_in_trial_m
fake_uuid,
):
mocker.patch('app.main.views.uploads.service_api_client')
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'The Queen'})
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'The Queen'}))
# client_request uses service_one, which is in trial mode
page = client_request.get(
@@ -603,7 +604,7 @@ def test_uploaded_letter_preview_image_400s_for_bad_page_type(
def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mocker, service_one, client_request):
metadata = {'filename': 'my_file.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'address'}
metadata = LetterMetadata({'filename': 'my_file.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'address'})
mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', metadata))
mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter')
@@ -656,8 +657,8 @@ def test_send_uploaded_letter_when_metadata_states_pdf_is_invalid(mocker, servic
mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter')
mocker.patch(
'app.main.views.uploads.get_letter_metadata',
return_value={'filename': 'my_file.pdf', 'page_count': '3', 'status': 'invalid',
'message': 'error', 'invalid_pages': '[1]'}
return_value=LetterMetadata({'filename': 'my_file.pdf', 'page_count': '3', 'status': 'invalid',
'message': 'error', 'invalid_pages': '[1]'})
)
service_one['permissions'] = ['letter', 'upload_letters']