From bb2dc2db66a5c31b73c2902bb66756bf8d40ffdd Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 9 Oct 2023 16:26:35 +0100 Subject: [PATCH 1/8] prmdr-242 create name validtions --- lambdas/models/nhs_document_reference.py | 6 ++++-- lambdas/services/lloyd_george_validator.py | 15 ++++++++++++++- .../unit/services/test_lloyd_george_validator.py | 16 ++++++++++++++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lambdas/models/nhs_document_reference.py b/lambdas/models/nhs_document_reference.py index 9153be2df..7fb194fec 100644 --- a/lambdas/models/nhs_document_reference.py +++ b/lambdas/models/nhs_document_reference.py @@ -4,7 +4,7 @@ from enums.metadata_field_names import DocumentReferenceMetadataFields from pydantic import BaseModel, model_validator -from services.lloyd_george_validator import validate_lg_file_type +from services.lloyd_george_validator import validate_lg_file_type, validate_file_name class UploadRequestDocument(BaseModel): @@ -14,8 +14,9 @@ class UploadRequestDocument(BaseModel): @model_validator(mode='before') @classmethod - def check_file_type_for_lg(cls, data: Any) -> Any: + def validation_for_lg(cls, data: Any) -> Any: if isinstance(data, dict): + file_name = data.get('fileName') doc_type = data.get('docType') content_type = data.get('contentType') elif isinstance(data, UploadRequestDocument): @@ -23,6 +24,7 @@ def check_file_type_for_lg(cls, data: Any) -> Any: content_type = data.contentType if doc_type == 'LG': validate_lg_file_type(content_type) + validate_file_name(file_name) return data diff --git a/lambdas/services/lloyd_george_validator.py b/lambdas/services/lloyd_george_validator.py index 6d3c43801..c612a27d8 100644 --- a/lambdas/services/lloyd_george_validator.py +++ b/lambdas/services/lloyd_george_validator.py @@ -1,7 +1,20 @@ +import re + + class LGFileTypeException(ValueError): - """One or more of the files do not match the required file type.""" + """One or more of the files do not match the required file type""" + pass + +class LGFileNameException(ValueError): + """One or more of the files do not match naming convention""" pass def validate_lg_file_type(file_type): if file_type != 'application/pdf': raise LGFileTypeException + +def validate_file_name(name): + lg_regex = '[0-9]+of[0-9]+_Lloyd_George_Record_\\[[A-Za-z]+\\s[A-Za-z]+]_\\[[0-9]{10}]_\\[\\d\\d-\\d\\d-\\d\\d\\d\\d].pdf' + if not re.fullmatch(lg_regex, name): + raise LGFileNameException + diff --git a/lambdas/tests/unit/services/test_lloyd_george_validator.py b/lambdas/tests/unit/services/test_lloyd_george_validator.py index 1e0dfdf66..c3ed2f0f5 100644 --- a/lambdas/tests/unit/services/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/services/test_lloyd_george_validator.py @@ -1,6 +1,6 @@ import pytest -from services.lloyd_george_validator import validate_lg_file_type, LGFileTypeException +from services.lloyd_george_validator import validate_lg_file_type, LGFileTypeException, LGFileNameException, validate_file_name def test_catching_error_when_file_type_not_pdf(): with pytest.raises(LGFileTypeException): @@ -12,4 +12,16 @@ def test_valid_file_type(): file_type = 'application/pdf' validate_lg_file_type(file_type) except LGFileTypeException: - assert False, 'One or more of the files do not match the required file type.' + assert False, 'One or more of the files do not match the required file type' + +def test_invalid_file_name(): + with pytest.raises(LGFileNameException): + file_name = 'bad_name' + validate_file_name(file_name) + +def test_valid_file_name(): + try: + file_name = '1of1_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' + validate_file_name(file_name) + except LGFileNameException: + assert False, 'One or more of the files do not match naming convention' From b3bd130c6bd5a84f963e53a935a4f6cb6ef092b1 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Mon, 9 Oct 2023 17:20:51 +0100 Subject: [PATCH 2/8] prmdr-242 added more validation and test --- lambdas/services/lloyd_george_validator.py | 18 ++++++++- .../services/test_lloyd_george_validator.py | 39 ++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/lambdas/services/lloyd_george_validator.py b/lambdas/services/lloyd_george_validator.py index c612a27d8..39fb9fddd 100644 --- a/lambdas/services/lloyd_george_validator.py +++ b/lambdas/services/lloyd_george_validator.py @@ -9,12 +9,26 @@ class LGFileNameException(ValueError): """One or more of the files do not match naming convention""" pass -def validate_lg_file_type(file_type): +class LGInvalidFilesException(Exception): + """One or more of the files are not valid""" + pass + +def validate_lg_file_type(file_type: str): if file_type != 'application/pdf': raise LGFileTypeException -def validate_file_name(name): +def validate_file_name(name: str): lg_regex = '[0-9]+of[0-9]+_Lloyd_George_Record_\\[[A-Za-z]+\\s[A-Za-z]+]_\\[[0-9]{10}]_\\[\\d\\d-\\d\\d-\\d\\d\\d\\d].pdf' if not re.fullmatch(lg_regex, name): raise LGFileNameException +def check_for_duplicate_files(file_list: list[str]): + if len(file_list) > len(set(file_list)): + raise LGInvalidFilesException + +def check_for_number_of_files_match_expected(file_name: str, total_files_number: str): + lg_number_regex = 'of[0-9]+' + expected_number_of_files = re.search(lg_number_regex, file_name) + if expected_number_of_files and not expected_number_of_files.group()[2:] == total_files_number: + raise LGInvalidFilesException + diff --git a/lambdas/tests/unit/services/test_lloyd_george_validator.py b/lambdas/tests/unit/services/test_lloyd_george_validator.py index c3ed2f0f5..74e634435 100644 --- a/lambdas/tests/unit/services/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/services/test_lloyd_george_validator.py @@ -1,6 +1,7 @@ import pytest -from services.lloyd_george_validator import validate_lg_file_type, LGFileTypeException, LGFileNameException, validate_file_name +from services.lloyd_george_validator import validate_lg_file_type, LGFileTypeException, LGFileNameException, LGInvalidFilesException, validate_file_name, check_for_duplicate_files, check_for_number_of_files_match_expected + def test_catching_error_when_file_type_not_pdf(): with pytest.raises(LGFileTypeException): @@ -25,3 +26,39 @@ def test_valid_file_name(): validate_file_name(file_name) except LGFileNameException: assert False, 'One or more of the files do not match naming convention' + +def test_files_with_duplication(): + with pytest.raises(LGInvalidFilesException): + lg_file_list = [ + '1of1_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf', + '1of1_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' + ] + check_for_duplicate_files(lg_file_list) + +def test_files_without_duplication(): + try: + lg_file_list = [ + '1of2_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf', + '2of2_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' + ] + check_for_duplicate_files(lg_file_list) + except LGFileNameException: + assert False, 'One or more of the files are not valid' + +def test_files_list_with_missing_files(): + with pytest.raises(LGInvalidFilesException): + lg_file_list = [ + '1of3_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf', + '2of3_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' + ] + check_for_number_of_files_match_expected(lg_file_list[0], str(len(lg_file_list))) + +def test_files_without_missing_files(): + try: + lg_file_list = [ + '1of2_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf', + '2of2_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' + ] + check_for_number_of_files_match_expected(lg_file_list[0], str(len(lg_file_list))) + except LGFileNameException: + assert False, 'One or more of the files are not valid' \ No newline at end of file From 5ad95d77972c509a9f8fcdb677ea27c105bc5f96 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Tue, 10 Oct 2023 12:15:57 +0100 Subject: [PATCH 3/8] prmdr-242 refactor validtion service and add unit test --- .../create_document_reference_handler.py | 10 ++++ lambdas/models/nhs_document_reference.py | 18 +----- lambdas/services/lloyd_george_validator.py | 16 ++++- .../test_create_document_reference_handler.py | 13 ++-- .../helpers/data/create_document_reference.py | 59 +++++++++++++++++-- 5 files changed, 84 insertions(+), 32 deletions(-) diff --git a/lambdas/handlers/create_document_reference_handler.py b/lambdas/handlers/create_document_reference_handler.py index 2cb032d8e..0afac3c47 100644 --- a/lambdas/handlers/create_document_reference_handler.py +++ b/lambdas/handlers/create_document_reference_handler.py @@ -16,6 +16,8 @@ from utils.lambda_response import ApiGatewayResponse from utils.utilities import validate_id +from services.lloyd_george_validator import validate_lg_files, LGInvalidFilesException, LGFileTypeException, LGFileNameException + sys.path.append(os.path.join(os.path.dirname(__file__))) logger = logging.getLogger() @@ -119,6 +121,7 @@ def lambda_handler(event, context): return response try: + s3_response = s3_service.create_document_presigned_url_handler( document_reference.s3_bucket_name, document_reference.nhs_number + "/" + document_reference.id, @@ -135,6 +138,7 @@ def lambda_handler(event, context): return response try: + validate_lg_files(lg_documents) dynamo_service = DynamoDBService() if arf_documents: logger.info("Writing ARF document references") @@ -152,6 +156,12 @@ def lambda_handler(event, context): 500, "An error occurred when creating document reference", "POST" ).create_api_gateway_response() return response + except (LGInvalidFilesException, LGFileTypeException, LGFileNameException) as e: + logger.error(e) + response = ApiGatewayResponse( + 400, "One or more if the files is not valid", "POST" + ).create_api_gateway_response() + return response return ApiGatewayResponse( 200, json.dumps(url_responses), "POST" diff --git a/lambdas/models/nhs_document_reference.py b/lambdas/models/nhs_document_reference.py index 7fb194fec..e2be32283 100644 --- a/lambdas/models/nhs_document_reference.py +++ b/lambdas/models/nhs_document_reference.py @@ -2,9 +2,8 @@ from typing import Any from enums.metadata_field_names import DocumentReferenceMetadataFields -from pydantic import BaseModel, model_validator +from pydantic import BaseModel -from services.lloyd_george_validator import validate_lg_file_type, validate_file_name class UploadRequestDocument(BaseModel): @@ -12,21 +11,6 @@ class UploadRequestDocument(BaseModel): contentType: str docType: str - @model_validator(mode='before') - @classmethod - def validation_for_lg(cls, data: Any) -> Any: - if isinstance(data, dict): - file_name = data.get('fileName') - doc_type = data.get('docType') - content_type = data.get('contentType') - elif isinstance(data, UploadRequestDocument): - doc_type = data.docType - content_type = data.contentType - if doc_type == 'LG': - validate_lg_file_type(content_type) - validate_file_name(file_name) - return data - class NHSDocumentReference: def __init__( diff --git a/lambdas/services/lloyd_george_validator.py b/lambdas/services/lloyd_george_validator.py index 39fb9fddd..2cabc130a 100644 --- a/lambdas/services/lloyd_george_validator.py +++ b/lambdas/services/lloyd_george_validator.py @@ -1,5 +1,7 @@ import re +from lambdas.models.nhs_document_reference import NHSDocumentReference + class LGFileTypeException(ValueError): """One or more of the files do not match the required file type""" @@ -18,7 +20,7 @@ def validate_lg_file_type(file_type: str): raise LGFileTypeException def validate_file_name(name: str): - lg_regex = '[0-9]+of[0-9]+_Lloyd_George_Record_\\[[A-Za-z]+\\s[A-Za-z]+]_\\[[0-9]{10}]_\\[\\d\\d-\\d\\d-\\d\\d\\d\\d].pdf' + lg_regex = r'[0-9]+of[0-9]+_Lloyd_George_Record_\[[A-Za-z]+\s[A-Za-z]+]_\[[0-9]{10}]_\[\d\d-\d\d-\d\d\d\d].pdf' if not re.fullmatch(lg_regex, name): raise LGFileNameException @@ -26,9 +28,17 @@ def check_for_duplicate_files(file_list: list[str]): if len(file_list) > len(set(file_list)): raise LGInvalidFilesException -def check_for_number_of_files_match_expected(file_name: str, total_files_number: str): +def check_for_number_of_files_match_expected(file_name: str, total_files_number: int): lg_number_regex = 'of[0-9]+' expected_number_of_files = re.search(lg_number_regex, file_name) - if expected_number_of_files and not expected_number_of_files.group()[2:] == total_files_number: + if expected_number_of_files and not expected_number_of_files.group()[2:] == str(total_files_number): raise LGInvalidFilesException +def validate_lg_files(file_list: list[NHSDocumentReference]): + files_name_list = [] + for doc in file_list: + check_for_number_of_files_match_expected(doc.file_name, len(file_list)) + validate_lg_file_type(doc.content_type) + validate_file_name(doc.file_name) + files_name_list.append(doc.file_name) + check_for_duplicate_files(files_name_list) diff --git a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py index 80c2a30bd..379671774 100644 --- a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py @@ -10,7 +10,8 @@ TEST_NHS_NUMBER, TEST_OBJECT_KEY) from tests.unit.helpers.data.create_document_reference import ( ARF_MOCK_EVENT_BODY, ARF_MOCK_RESPONSE, LG_AND_ARF_MOCK_RESPONSE, - LG_MOCK_EVENT_BODY, LG_MOCK_RESPONSE, MOCK_EVENT_BODY, LG_MOCK_BAD_EVENT_BODY) + LG_MOCK_EVENT_BODY, LG_MOCK_RESPONSE, MOCK_EVENT_BODY, LG_MOCK_BAD_FILE_TYPE_EVENT_BODY, + LG_MOCK_MISSING_FILES_EVENT_BODY, LG_MOCK_BAD_FILE_NAME_EVENT_BODY, LG_MOCK_DUPLICATE_FILES_EVENT_BODY) from tests.unit.services.test_s3_service import MOCK_PRESIGNED_POST_RESPONSE from utils.lambda_response import ApiGatewayResponse @@ -265,14 +266,14 @@ def test_create_document_reference_arf_type_s3_ClientError_returns_500( actual = lambda_handler(arf_type_event, context) assert actual == expected - -def test_invalid_file_type_for_lg_return_400(set_env, context): +@pytest.mark.parametrize("event_body", [LG_MOCK_BAD_FILE_TYPE_EVENT_BODY, LG_MOCK_MISSING_FILES_EVENT_BODY, LG_MOCK_BAD_FILE_NAME_EVENT_BODY, LG_MOCK_DUPLICATE_FILES_EVENT_BODY]) +def test_invalid_file_type_for_lg_return_400(set_env, context, event_body): expected = ApiGatewayResponse( 400, - "Failed to parse document upload request data", - "GET", + "One or more if the files is not valid", + "POST", ).create_api_gateway_response() - actual = lambda_handler({"body": json.dumps(LG_MOCK_BAD_EVENT_BODY)}, context) + actual = lambda_handler({"body": json.dumps(event_body)}, context) assert actual == expected def test_create_document_reference_unknown_document_type_returns_400( diff --git a/lambdas/tests/unit/helpers/data/create_document_reference.py b/lambdas/tests/unit/helpers/data/create_document_reference.py index 4cba8720d..a713cce6d 100644 --- a/lambdas/tests/unit/helpers/data/create_document_reference.py +++ b/lambdas/tests/unit/helpers/data/create_document_reference.py @@ -69,33 +69,80 @@ "created": "2023-10-02T15:55:30.650Z", } -LG_MOCK_BAD_EVENT_BODY = { +LG_MOCK_BAD_FILE_TYPE_EVENT_BODY = { "resourceType": "DocumentReference", "subject": {"identifier": {"value": TEST_NHS_NUMBER}}, "content": [ { "attachment": [ { - "fileName": f"1of3_Lloyd_George_Record_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", + "fileName": f"1of1_Lloyd_George_Record_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", "contentType": "text/plain", "docType": "LG", - }, + } + ] + } + ], + "created": "2023-10-02T15:55:30.650Z", +} + +LG_MOCK_BAD_FILE_NAME_EVENT_BODY = { + "resourceType": "DocumentReference", + "subject": {"identifier": {"value": TEST_NHS_NUMBER}}, + "content": [ + { + "attachment": [ { - "fileName": f"2of3_Lloyd_George_Record_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", + "fileName": f"1of1_BAD_NAME_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", "contentType": "application/pdf", "docType": "LG", - }, + } + ] + } + ], + "created": "2023-10-02T15:55:30.650Z", +} + +LG_MOCK_MISSING_FILES_EVENT_BODY = { + "resourceType": "DocumentReference", + "subject": {"identifier": {"value": TEST_NHS_NUMBER}}, + "content": [ + { + "attachment": [ { - "fileName": f"3of3_Lloyd_George_Record_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", + "fileName": f"1of3_Lloyd_George_Record_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", + "contentType": "application/pdf", + "docType": "LG", + } + ] + } + ], + "created": "2023-10-02T15:55:30.650Z", +} + +LG_MOCK_DUPLICATE_FILES_EVENT_BODY = { + "resourceType": "DocumentReference", + "subject": {"identifier": {"value": TEST_NHS_NUMBER}}, + "content": [ + { + "attachment": [ + { + "fileName": f"1of2_Lloyd_George_Record_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", "contentType": "application/pdf", "docType": "LG", }, + { + "fileName": f"1of2_Lloyd_George_Record_[Joe Bloggs]_[{TEST_NHS_NUMBER}]_[25-12-2019].pdf", + "contentType": "application/pdf", + "docType": "LG", + } ] } ], "created": "2023-10-02T15:55:30.650Z", } + ARF_MOCK_EVENT_BODY = { "resourceType": "DocumentReference", "subject": {"identifier": {"value": TEST_NHS_NUMBER}}, From 96dd0a038d115c99c821a64802a5243e6e473c9e Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Tue, 10 Oct 2023 12:47:16 +0100 Subject: [PATCH 4/8] prmdr-242 test-fix --- .../test_create_document_reference_handler.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py index 379671774..2f17337b0 100644 --- a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py @@ -267,7 +267,18 @@ def test_create_document_reference_arf_type_s3_ClientError_returns_500( assert actual == expected @pytest.mark.parametrize("event_body", [LG_MOCK_BAD_FILE_TYPE_EVENT_BODY, LG_MOCK_MISSING_FILES_EVENT_BODY, LG_MOCK_BAD_FILE_NAME_EVENT_BODY, LG_MOCK_DUPLICATE_FILES_EVENT_BODY]) -def test_invalid_file_type_for_lg_return_400(set_env, context, event_body): +def test_invalid_file_type_for_lg_return_400(set_env, context, mocker, event_body): + mock_supported_document_get_from_field_name = mocker.patch( + "enums.supported_document_types.SupportedDocumentTypes.get_from_field_name" + ) + mock_supported_document_get_from_field_name.return_value = SupportedDocumentTypes.LG + mocker.patch("services.dynamo_service.DynamoDBService.post_item_service") + mock_presigned = mocker.patch( + "services.s3_service.S3Service.create_document_presigned_url_handler" + ) + mock_presigned.return_value = MOCK_PRESIGNED_POST_RESPONSE + + expected = ApiGatewayResponse( 400, "One or more if the files is not valid", From f6b8901a69f309b36388273a5f09e84255c6d260 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Tue, 10 Oct 2023 14:48:22 +0100 Subject: [PATCH 5/8] prmdr-242 change regex --- lambdas/services/lloyd_george_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/services/lloyd_george_validator.py b/lambdas/services/lloyd_george_validator.py index 2cabc130a..3802b15b0 100644 --- a/lambdas/services/lloyd_george_validator.py +++ b/lambdas/services/lloyd_george_validator.py @@ -20,7 +20,7 @@ def validate_lg_file_type(file_type: str): raise LGFileTypeException def validate_file_name(name: str): - lg_regex = r'[0-9]+of[0-9]+_Lloyd_George_Record_\[[A-Za-z]+\s[A-Za-z]+]_\[[0-9]{10}]_\[\d\d-\d\d-\d\d\d\d].pdf' + lg_regex = r'[0-9]+of[0-9]+_Lloyd_George_Record_\[[A-Za-z ]+]_\[[0-9]{10}]_\[\d\d-\d\d-\d\d\d\d].pdf' if not re.fullmatch(lg_regex, name): raise LGFileNameException From 93af8489b3201ea1cc0f5c39485c86fe97e53b4d Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Wed, 11 Oct 2023 15:16:13 +0100 Subject: [PATCH 6/8] prmdr-242 Pr changes --- .../create_document_reference_handler.py | 4 ++-- lambdas/services/lloyd_george_validator.py | 18 ++++-------------- .../services/test_lloyd_george_validator.py | 18 +++++++++--------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/lambdas/handlers/create_document_reference_handler.py b/lambdas/handlers/create_document_reference_handler.py index 0afac3c47..9c67197ca 100644 --- a/lambdas/handlers/create_document_reference_handler.py +++ b/lambdas/handlers/create_document_reference_handler.py @@ -16,7 +16,7 @@ from utils.lambda_response import ApiGatewayResponse from utils.utilities import validate_id -from services.lloyd_george_validator import validate_lg_files, LGInvalidFilesException, LGFileTypeException, LGFileNameException +from services.lloyd_george_validator import validate_lg_files, LGInvalidFilesException sys.path.append(os.path.join(os.path.dirname(__file__))) @@ -156,7 +156,7 @@ def lambda_handler(event, context): 500, "An error occurred when creating document reference", "POST" ).create_api_gateway_response() return response - except (LGInvalidFilesException, LGFileTypeException, LGFileNameException) as e: + except LGInvalidFilesException as e: logger.error(e) response = ApiGatewayResponse( 400, "One or more if the files is not valid", "POST" diff --git a/lambdas/services/lloyd_george_validator.py b/lambdas/services/lloyd_george_validator.py index 3802b15b0..75ac6c0fb 100644 --- a/lambdas/services/lloyd_george_validator.py +++ b/lambdas/services/lloyd_george_validator.py @@ -2,37 +2,27 @@ from lambdas.models.nhs_document_reference import NHSDocumentReference - -class LGFileTypeException(ValueError): - """One or more of the files do not match the required file type""" - pass - -class LGFileNameException(ValueError): - """One or more of the files do not match naming convention""" - pass - class LGInvalidFilesException(Exception): - """One or more of the files are not valid""" pass def validate_lg_file_type(file_type: str): if file_type != 'application/pdf': - raise LGFileTypeException + raise LGInvalidFilesException('One or more of the files do not match the required file type') def validate_file_name(name: str): lg_regex = r'[0-9]+of[0-9]+_Lloyd_George_Record_\[[A-Za-z ]+]_\[[0-9]{10}]_\[\d\d-\d\d-\d\d\d\d].pdf' if not re.fullmatch(lg_regex, name): - raise LGFileNameException + raise LGInvalidFilesException('One or more of the files do not match naming convention') def check_for_duplicate_files(file_list: list[str]): if len(file_list) > len(set(file_list)): - raise LGInvalidFilesException + raise LGInvalidFilesException('One or more of the files has the same filename') def check_for_number_of_files_match_expected(file_name: str, total_files_number: int): lg_number_regex = 'of[0-9]+' expected_number_of_files = re.search(lg_number_regex, file_name) if expected_number_of_files and not expected_number_of_files.group()[2:] == str(total_files_number): - raise LGInvalidFilesException + raise LGInvalidFilesException('There are missing file(s) in the request') def validate_lg_files(file_list: list[NHSDocumentReference]): files_name_list = [] diff --git a/lambdas/tests/unit/services/test_lloyd_george_validator.py b/lambdas/tests/unit/services/test_lloyd_george_validator.py index 74e634435..13b093f86 100644 --- a/lambdas/tests/unit/services/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/services/test_lloyd_george_validator.py @@ -1,10 +1,10 @@ import pytest -from services.lloyd_george_validator import validate_lg_file_type, LGFileTypeException, LGFileNameException, LGInvalidFilesException, validate_file_name, check_for_duplicate_files, check_for_number_of_files_match_expected +from services.lloyd_george_validator import validate_lg_file_type, LGInvalidFilesException, validate_file_name, check_for_duplicate_files, check_for_number_of_files_match_expected def test_catching_error_when_file_type_not_pdf(): - with pytest.raises(LGFileTypeException): + with pytest.raises(LGInvalidFilesException): file_type = 'image/png' validate_lg_file_type(file_type) @@ -12,11 +12,11 @@ def test_valid_file_type(): try: file_type = 'application/pdf' validate_lg_file_type(file_type) - except LGFileTypeException: + except LGInvalidFilesException: assert False, 'One or more of the files do not match the required file type' def test_invalid_file_name(): - with pytest.raises(LGFileNameException): + with pytest.raises(LGInvalidFilesException): file_name = 'bad_name' validate_file_name(file_name) @@ -24,7 +24,7 @@ def test_valid_file_name(): try: file_name = '1of1_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' validate_file_name(file_name) - except LGFileNameException: + except LGInvalidFilesException: assert False, 'One or more of the files do not match naming convention' def test_files_with_duplication(): @@ -42,8 +42,8 @@ def test_files_without_duplication(): '2of2_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' ] check_for_duplicate_files(lg_file_list) - except LGFileNameException: - assert False, 'One or more of the files are not valid' + except LGInvalidFilesException: + assert False, 'One or more of the files has the same filename' def test_files_list_with_missing_files(): with pytest.raises(LGInvalidFilesException): @@ -60,5 +60,5 @@ def test_files_without_missing_files(): '2of2_Lloyd_George_Record_[Joe Bloggs]_[1111111111]_[25-12-2019].pdf' ] check_for_number_of_files_match_expected(lg_file_list[0], str(len(lg_file_list))) - except LGFileNameException: - assert False, 'One or more of the files are not valid' \ No newline at end of file + except LGInvalidFilesException: + assert False, 'There are missing file(s) in the request' From 48a78d6bfa291bbc8f4e7fe4b646b0eb917da396 Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Thu, 12 Oct 2023 16:56:12 +0100 Subject: [PATCH 7/8] allowing special characters to regex --- lambdas/handlers/create_document_reference_handler.py | 2 +- lambdas/services/lloyd_george_validator.py | 2 +- .../unit/handlers/test_create_document_reference_handler.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lambdas/handlers/create_document_reference_handler.py b/lambdas/handlers/create_document_reference_handler.py index 9c67197ca..815e7ff35 100644 --- a/lambdas/handlers/create_document_reference_handler.py +++ b/lambdas/handlers/create_document_reference_handler.py @@ -159,7 +159,7 @@ def lambda_handler(event, context): except LGInvalidFilesException as e: logger.error(e) response = ApiGatewayResponse( - 400, "One or more if the files is not valid", "POST" + 400, "One or more of the files is not valid", "POST" ).create_api_gateway_response() return response diff --git a/lambdas/services/lloyd_george_validator.py b/lambdas/services/lloyd_george_validator.py index 75ac6c0fb..00d9bb67a 100644 --- a/lambdas/services/lloyd_george_validator.py +++ b/lambdas/services/lloyd_george_validator.py @@ -10,7 +10,7 @@ def validate_lg_file_type(file_type: str): raise LGInvalidFilesException('One or more of the files do not match the required file type') def validate_file_name(name: str): - lg_regex = r'[0-9]+of[0-9]+_Lloyd_George_Record_\[[A-Za-z ]+]_\[[0-9]{10}]_\[\d\d-\d\d-\d\d\d\d].pdf' + lg_regex = r'[0-9]+of[0-9]+_Lloyd_George_Record_\[[A-Za-z À-ÿ\-]+]_\[[0-9]{10}]_\[\d\d-\d\d-\d\d\d\d].pdf' if not re.fullmatch(lg_regex, name): raise LGInvalidFilesException('One or more of the files do not match naming convention') diff --git a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py index 2f17337b0..ea8110d4d 100644 --- a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py @@ -281,7 +281,7 @@ def test_invalid_file_type_for_lg_return_400(set_env, context, mocker, event_bod expected = ApiGatewayResponse( 400, - "One or more if the files is not valid", + "One or more of the files is not valid", "POST", ).create_api_gateway_response() actual = lambda_handler({"body": json.dumps(event_body)}, context) From b6a55882d780bef4eb1e86da98c3b6ec8e3bf09e Mon Sep 17 00:00:00 2001 From: NogaNHS Date: Thu, 12 Oct 2023 17:02:58 +0100 Subject: [PATCH 8/8] added test for allowing special char --- lambdas/tests/unit/services/test_lloyd_george_validator.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lambdas/tests/unit/services/test_lloyd_george_validator.py b/lambdas/tests/unit/services/test_lloyd_george_validator.py index 13b093f86..9a3415c25 100644 --- a/lambdas/tests/unit/services/test_lloyd_george_validator.py +++ b/lambdas/tests/unit/services/test_lloyd_george_validator.py @@ -27,6 +27,13 @@ def test_valid_file_name(): except LGInvalidFilesException: assert False, 'One or more of the files do not match naming convention' +def test_valid_file_name_special_characters(): + try: + file_name = '1of1_Lloyd_George_Record_[Joé Blöggês-Glüë]_[1111111111]_[25-12-2019].pdf' + validate_file_name(file_name) + except LGInvalidFilesException: + assert False, 'One or more of the files do not match naming convention' + def test_files_with_duplication(): with pytest.raises(LGInvalidFilesException): lg_file_list = [