diff --git a/backend/src/fhir_service.py b/backend/src/fhir_service.py index 43e221a56..996725993 100644 --- a/backend/src/fhir_service.py +++ b/backend/src/fhir_service.py @@ -332,11 +332,6 @@ def search_immunizations( Bundles the resources with the relevant patient resource and returns the bundle along with a boolean to state whether the supplier requested vaccine types they were not authorised for. """ - # TODO: is disease type a mandatory field? (I assumed it is) - # i.e. Should we provide a search option for getting Patient's entire imms history? - if not nhs_number_mod11_check(nhs_number): - return create_diagnostics(), False - permitted_vacc_types = self.authoriser.filter_permitted_vacc_types( supplier_system, ApiOperationCode.SEARCH, set(vaccine_types) ) diff --git a/backend/src/parameter_parser.py b/backend/src/parameter_parser.py index 7c5419c90..1a88d56c0 100644 --- a/backend/src/parameter_parser.py +++ b/backend/src/parameter_parser.py @@ -8,6 +8,7 @@ from clients import redis_client, logger from models.errors import ParameterException +from models.utils.generic_utils import nhs_number_mod11_check from models.constants import Constants ParamValue = list[str] @@ -35,6 +36,110 @@ class SearchParams: def __repr__(self): return str(self.__dict__) +def process_patient_identifier(identifier_params: ParamContainer) -> str: + """Validate and parse patient identifier parameter. + + :raises ParameterException: + """ + patient_identifiers = identifier_params.get(patient_identifier_key, []) + patient_identifier = patient_identifiers[0] if len(patient_identifiers) == 1 else None + + if patient_identifier is None: + raise ParameterException(f"Search parameter {patient_identifier_key} must have one value.") + + patient_identifier_parts = patient_identifier.split("|") + identifier_system = patient_identifier_parts[0] + if len(patient_identifier_parts) != 2 or identifier_system != patient_identifier_system: + raise ParameterException("patient.identifier must be in the format of " + f"\"{patient_identifier_system}|{{NHS number}}\" " + f"e.g. \"{patient_identifier_system}|9000000009\"") + + nhs_number = patient_identifier_parts[1] + if not nhs_number_mod11_check(nhs_number): + raise ParameterException("Search parameter patient.identifier must be a valid NHS number.") + + return nhs_number + + +def process_immunization_target(imms_params: ParamContainer) -> list[str]: + """Validate and parse immunization target parameter. + + :raises ParameterException: + """ + vaccine_types = [vaccine_type for vaccine_type in set(imms_params.get(immunization_target_key, [])) if + vaccine_type is not None] + if len(vaccine_types) < 1: + raise ParameterException(f"Search parameter {immunization_target_key} must have one or more values.") + + valid_vaccine_types = redis_client.hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY) + if any(x not in valid_vaccine_types for x in vaccine_types): + raise ParameterException( + f"immunization-target must be one or more of the following: {', '.join(valid_vaccine_types)}") + + return vaccine_types + + +def process_mandatory_params(params: ParamContainer) -> tuple[str, list[str]]: + """Validate mandatory params and return (patient_identifier, vaccine_types). + Raises ParameterException for any validation error. + """ + # patient.identifier + patient_identifier = process_patient_identifier(params) + + # immunization.target + vaccine_types = process_immunization_target(params) + + return patient_identifier, vaccine_types + + +def process_optional_params(params: ParamContainer) -> tuple[datetime.date, datetime.date, Optional[str], list[str]]: + """Parse optional params (date.from, date.to, _include). + Returns (date_from, date_to, include, errors). + """ + errors: list[str] = [] + date_from = None + date_to = None + + date_froms = params.get(date_from_key, []) + if len(date_froms) > 1: + errors.append(f"Search parameter {date_from_key} may have one value at most.") + + try: + date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date() if len(date_froms) == 1 else date_from_default + except ValueError: + errors.append(f"Search parameter {date_from_key} must be in format: YYYY-MM-DD") + + date_tos = params.get(date_to_key, []) + if len(date_tos) > 1: + errors.append(f"Search parameter {date_to_key} may have one value at most.") + + try: + date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date() if len(date_tos) == 1 else date_to_default + except ValueError: + errors.append(f"Search parameter {date_to_key} must be in format: YYYY-MM-DD") + + includes = params.get(include_key, []) + if includes and includes[0].lower() != "immunization:patient": + errors.append(f"Search parameter {include_key} may only be 'Immunization:patient' if provided.") + include = includes[0] if len(includes) > 0 else None + + return date_from, date_to, include, errors + +def process_search_params(params: ParamContainer) -> SearchParams: + """Validate and parse search parameters. + :raises ParameterException: + """ + patient_identifier, vaccine_types = process_mandatory_params(params) + date_from, date_to, include, errors = process_optional_params(params) + + if date_from and date_to and date_from > date_to: + errors.append(f"Search parameter {date_from_key} must be before {date_to_key}") + + if errors: + raise ParameterException("; ".join(errors)) + + return SearchParams(patient_identifier, vaccine_types, date_from, date_to, include) + def process_params(aws_event: APIGatewayProxyEventV1) -> ParamContainer: """Combines query string and content parameters. Duplicates not allowed. Splits on a comma.""" @@ -80,74 +185,6 @@ def parse_body_params(aws_event: APIGatewayProxyEventV1) -> ParamContainer: return parsed_params -def process_search_params(params: ParamContainer) -> SearchParams: - """Validate and parse search parameters. - - :raises ParameterException: - """ - - # patient.identifier - patient_identifiers = params.get(patient_identifier_key, []) - patient_identifier = patient_identifiers[0] if len(patient_identifiers) == 1 else None - - if patient_identifier is None: - raise ParameterException(f"Search parameter {patient_identifier_key} must have one value.") - - patient_identifier_parts = patient_identifier.split("|") - if len(patient_identifier_parts) != 2 or not patient_identifier_parts[ - 0] == patient_identifier_system: - raise ParameterException("patient.identifier must be in the format of " - f"\"{patient_identifier_system}|{{NHS number}}\" " - f"e.g. \"{patient_identifier_system}|9000000009\"") - - patient_identifier = patient_identifier.split("|")[1] - - # immunization.target - params[immunization_target_key] = list(set(params.get(immunization_target_key, []))) - vaccine_types = [vaccine_type for vaccine_type in params[immunization_target_key] if - vaccine_type is not None] - if len(vaccine_types) < 1: - raise ParameterException(f"Search parameter {immunization_target_key} must have one or more values.") - - valid_vaccine_types = redis_client.hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY) - if any(x not in valid_vaccine_types for x in vaccine_types): - raise ParameterException( - f"immunization-target must be one or more of the following: {', '.join(valid_vaccine_types)}") - - # date.from - date_froms = params.get(date_from_key, []) - - if len(date_froms) > 1: - raise ParameterException(f"Search parameter {date_from_key} may have one value at most.") - - try: - date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date() \ - if len(date_froms) == 1 else date_from_default - except ValueError: - raise ParameterException(f"Search parameter {date_from_key} must be in format: YYYY-MM-DD") - - # date.to - date_tos = params.get(date_to_key, []) - - if len(date_tos) > 1: - raise ParameterException(f"Search parameter {date_to_key} may have one value at most.") - - try: - date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date() \ - if len(date_tos) == 1 else date_to_default - except ValueError: - raise ParameterException(f"Search parameter {date_to_key} must be in format: YYYY-MM-DD") - - if date_from and date_to and date_from > date_to: - raise ParameterException(f"Search parameter {date_from_key} must be before {date_to_key}") - - # include - includes = params.get(include_key, []) - include = includes[0] if len(includes) > 0 else None - - return SearchParams(patient_identifier, vaccine_types, date_from, date_to, include) - - def create_query_string(search_params: SearchParams) -> str: params = [ (immunization_target_key, ",".join(map(quote, search_params.immunization_targets))), diff --git a/backend/tests/test_parameter_parser.py b/backend/tests/test_parameter_parser.py index 4ad49a935..d2aa3555f 100644 --- a/backend/tests/test_parameter_parser.py +++ b/backend/tests/test_parameter_parser.py @@ -11,6 +11,8 @@ process_params, process_search_params, create_query_string, + create_query_string, + include_key, SearchParams, ) @@ -106,7 +108,7 @@ def test_process_search_params_checks_patient_identifier_format(self): self.immunization_target_key: ["RSV"], } ) - self.assertIsNotNone(params) + def test_process_search_params_whitelists_immunization_target(self): mock_redis_key = "RSV" @@ -115,7 +117,7 @@ def test_process_search_params_whitelists_immunization_target(self): process_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], - self.immunization_target_key: ["not-a-code"], + self.immunization_target_key: ["FLU", "COVID-19", "NOT-A-REAL-VALUE"], } ) self.assertEqual( @@ -123,6 +125,10 @@ def test_process_search_params_whitelists_immunization_target(self): f"Unexpected exception message: {str(e.exception)}" ) + + def test_process_search_params_immunization_target(self): + mock_redis_key = "RSV" + self.mock_redis_client.hkeys.return_value = [mock_redis_key] params = process_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], @@ -132,6 +138,7 @@ def test_process_search_params_whitelists_immunization_target(self): self.assertIsNotNone(params) + def test_search_params_date_from_must_be_before_date_to(self): self.mock_redis_client.hkeys.return_value = ["RSV"] params = process_search_params( @@ -210,3 +217,83 @@ def test_create_query_string_with_multiple_immunization_targets_comma_separated( expected = "-immunization.target=b,c&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7Ca" self.assertEqual(expected, query_string) + + def test_process_search_params_dedupes_immunization_targets_and_respects_include(self): + """Ensure duplicate immunization targets are deduped and include is preserved.""" + self.mock_redis_client.hkeys.return_value = ["RSV", "FLU"] + + params = process_search_params( + { + self.patient_identifier_key: [ + "https://fhir.nhs.uk/Id/nhs-number|9000000009" + ], + self.immunization_target_key: ["RSV", "RSV", "FLU"], + "_include": ["immunization:patient"], + } + ) + + # immunization targets should be deduped and preserve valid entries + self.assertIsInstance(params.immunization_targets, list) + self.assertCountEqual(params.immunization_targets, ["RSV", "FLU"]) + + # include should be returned as provided + self.assertEqual(params.include, "immunization:patient") + + def test_process_search_params_aggregates_date_errors(self): + """When multiple date-related errors occur they should be returned together.""" + self.mock_redis_client.hkeys.return_value = ["RSV"] + + with self.assertRaises(ParameterException) as e: + process_search_params( + { + self.patient_identifier_key: [ + "https://fhir.nhs.uk/Id/nhs-number|9000000009" + ], + self.immunization_target_key: ["RSV"], + self.date_from_key: ["2021-01-01", "2021-01-02"], # too many values + self.date_to_key: ["invalid-date"], # invalid format + } + ) + + expected = ( + f"Search parameter {self.date_from_key} may have one value at most.; " + f"Search parameter {self.date_to_key} must be in format: YYYY-MM-DD" + ) + self.assertEqual(str(e.exception), expected) + + def test_process_search_params_invalid_nhs_number_is_rejected(self): + """If the NHS number fails mod11 check a ParameterException is raised.""" + # redis returns a valid vaccine type + self.mock_redis_client.hkeys.return_value = ["RSV"] + + with self.assertRaises(ParameterException) as e: + process_search_params( + { + self.patient_identifier_key: [ + "https://fhir.nhs.uk/Id/nhs-number|1234567890" # invalid mod11 + ], + self.immunization_target_key: ["RSV"], + } + ) + + self.assertEqual(str(e.exception), f"Search parameter {self.patient_identifier_key} must be a valid NHS number.") + + def test_process_search_params_invalid_include_value_is_rejected(self): + """_include may only be 'Immunization:patient' if provided.""" + self.mock_redis_client.hkeys.return_value = ["RSV"] + + with self.assertRaises(ParameterException) as e: + process_search_params( + { + self.patient_identifier_key: [ + "https://fhir.nhs.uk/Id/nhs-number|9000000009" + ], + self.immunization_target_key: ["RSV"], + "_include": ["Patient:practitioner"], + } + ) + + self.assertEqual( + str(e.exception), + f"Search parameter {include_key} may only be 'Immunization:patient' if provided.", + )