diff --git a/infrastructure/instance/oas.yaml b/infrastructure/instance/oas.yaml index 409c375741..aff3dc4d15 100644 --- a/infrastructure/instance/oas.yaml +++ b/infrastructure/instance/oas.yaml @@ -128,28 +128,6 @@ paths: httpMethod: "POST" timeoutInMillis: 30000 type: "AWS_PROXY" - parameters: - - name: patient.identifier - in: query - required: true - schema: - type: string - - name: -immunization.target - in: query - schema: - type: string - - name: -date.from - in: query - schema: - type: string - - name: -date.to - in: query - schema: - type: string - - name: _include - in: query - schema: - type: string responses: "201": description: An Immunisation search event diff --git a/lambdas/backend/src/controller/aws_apig_event_utils.py b/lambdas/backend/src/controller/aws_apig_event_utils.py index 02a890c0a9..c331a186c3 100644 --- a/lambdas/backend/src/controller/aws_apig_event_utils.py +++ b/lambdas/backend/src/controller/aws_apig_event_utils.py @@ -9,6 +9,10 @@ from utils import dict_utils +def get_multi_value_query_params(event: APIGatewayProxyEventV1) -> dict: + return dict_utils.get_field(dict(event), "multiValueQueryStringParameters", default={}) + + def get_path_parameter(event: APIGatewayProxyEventV1, param_name: str) -> str: return dict_utils.get_field(event["pathParameters"], param_name, default="") diff --git a/lambdas/backend/src/controller/constants.py b/lambdas/backend/src/controller/constants.py index 868f0cc361..e3cd2ff488 100644 --- a/lambdas/backend/src/controller/constants.py +++ b/lambdas/backend/src/controller/constants.py @@ -1,4 +1,31 @@ """FHIR Controller constants""" +from enum import StrEnum + SUPPLIER_SYSTEM_HEADER_NAME = "SupplierSystem" E_TAG_HEADER_NAME = "E-Tag" + +SEARCH_IMMS_POST_PATH = "/Immunization/_search" + + +class IdentifierSearchParameterName(StrEnum): + IDENTIFIER = "identifier" + ELEMENTS = "_elements" + + +class ImmunizationSearchParameterName(StrEnum): + PATIENT_IDENTIFIER = "patient.identifier" + IMMUNIZATION_TARGET = "-immunization.target" + DATE_FROM = "-date.from" + DATE_TO = "-date.to" + INCLUDE = "_include" + + +class IdentifierSearchElement(StrEnum): + """Valid elements which can be requested to include in the identifier search response""" + + ID = "id" + META = "meta" + + +IMMUNIZATION_TARGET_LEGACY_KEY_NAME = "immunization-target" diff --git a/lambdas/backend/src/controller/fhir_api_exception_handler.py b/lambdas/backend/src/controller/fhir_api_exception_handler.py index 7bd3db71c0..349adf51aa 100644 --- a/lambdas/backend/src/controller/fhir_api_exception_handler.py +++ b/lambdas/backend/src/controller/fhir_api_exception_handler.py @@ -21,8 +21,10 @@ InvalidJsonError, InvalidResourceVersionError, InvalidStoredDataError, + ParameterExceptionError, ResourceVersionNotProvidedError, Severity, + TooManyResultsError, UnauthorizedError, UnauthorizedVaxError, UnhandledResponseError, @@ -38,6 +40,8 @@ InvalidResourceVersionError: 400, CustomValidationError: 400, ResourceVersionNotProvidedError: 400, + ParameterExceptionError: 400, + TooManyResultsError: 400, UnauthorizedError: 403, UnauthorizedVaxError: 403, ResourceNotFoundError: 404, diff --git a/lambdas/backend/src/controller/fhir_controller.py b/lambdas/backend/src/controller/fhir_controller.py index 580b7f2e8c..43025b6509 100644 --- a/lambdas/backend/src/controller/fhir_controller.py +++ b/lambdas/backend/src/controller/fhir_controller.py @@ -2,36 +2,36 @@ import json import os import re -import urllib.parse -import uuid from decimal import Decimal from json import JSONDecodeError -from typing import Optional +from urllib.parse import parse_qs from aws_lambda_typing.events import APIGatewayProxyEventV1 +from fhir.resources.R4B.bundle import Bundle +from fhir.resources.R4B.identifier import Identifier -from common.models.utils.generic_utils import check_keys_in_sources +from constants import MAX_RESPONSE_SIZE_BYTES from controller.aws_apig_event_utils import ( + get_multi_value_query_params, get_path_parameter, get_resource_version_header, get_supplier_system_header, ) from controller.aws_apig_response_utils import create_response -from controller.constants import E_TAG_HEADER_NAME +from controller.constants import E_TAG_HEADER_NAME, IdentifierSearchParameterName from controller.fhir_api_exception_handler import fhir_api_exception_handler +from controller.parameter_parser import ( + parse_search_params, + validate_and_retrieve_identifier_search_params, + validate_and_retrieve_search_params, +) from models.errors import ( - Code, InconsistentIdError, InvalidImmunizationIdError, InvalidJsonError, InvalidResourceVersionError, - ParameterExceptionError, - Severity, - UnauthorizedError, - UnauthorizedVaxError, - create_operation_outcome, + TooManyResultsError, ) -from parameter_parser import create_query_string, process_params, process_search_params from repository.fhir_repository import ImmunizationRepository, create_table from service.fhir_service import FhirService, get_service_url @@ -59,42 +59,6 @@ def __init__( ): self.fhir_service = fhir_service - def get_immunization_by_identifier(self, aws_event) -> dict: - try: - if aws_event.get("headers"): - query_params = aws_event.get("queryStringParameters", {}) - else: - raise UnauthorizedError() - except UnauthorizedError as unauthorized: - return create_response(403, unauthorized.to_operation_outcome()) - body = aws_event["body"] - if query_params and body: - error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.invalid, - diagnostics=('Parameters may not be duplicated. Use commas for "or".'), - ) - return create_response(400, error) - identifier, element, not_required, has_imms_identifier, has_element = self.fetch_identifier_system_and_element( - aws_event - ) - if not_required: - return self.create_response_for_identifier(not_required, has_imms_identifier, has_element) - # If not found, retrieve from multiValueQueryStringParameters - if id_error := self._validate_identifier_system(identifier, element): - return create_response(400, id_error) - identifiers = identifier.replace("|", "#") - supplier_system = self._identify_supplier_system(aws_event) - - try: - if resource := self.fhir_service.get_immunization_by_identifier( - identifiers, supplier_system, identifier, element - ): - return create_response(200, resource) - except UnauthorizedVaxError as unauthorized: - return create_response(403, unauthorized.to_operation_outcome()) - @fhir_api_exception_handler def get_immunization_by_id(self, aws_event: APIGatewayProxyEventV1) -> dict: imms_id = get_path_parameter(aws_event, "id") @@ -164,207 +128,99 @@ def delete_immunization(self, aws_event: APIGatewayProxyEventV1) -> dict: return create_response(204) - def search_immunizations(self, aws_event: APIGatewayProxyEventV1) -> dict: - try: - search_params = process_search_params(process_params(aws_event)) - except ParameterExceptionError as e: - return self._create_bad_request(e.message) - if search_params is None: - raise ParameterExceptionError("Failed to parse parameters.") + @fhir_api_exception_handler + def search_immunizations(self, aws_event: APIGatewayProxyEventV1, is_post_endpoint_req: bool = False) -> dict: + """Performs the client search request based on the parameters provided. The available searches are: + 1. Search by identifier: (more like a GET) retrieves immunisation by local identifier. + 2. Search by patient and immunisation target""" + search_params = self._get_search_params_from_request(aws_event, is_post_endpoint_req) + parsed_search_params = parse_search_params(search_params) + supplier_system = get_supplier_system_header(aws_event) - # Check vaxx type permissions- start - try: - if aws_event.get("headers"): - supplier_system = self._identify_supplier_system(aws_event) - else: - raise UnauthorizedError() - except UnauthorizedError as unauthorized: - return create_response(403, unauthorized.to_operation_outcome()) + if self._is_identifier_search(parsed_search_params): + return self._get_immunization_by_identifier(parsed_search_params, supplier_system) - try: - result, request_contained_unauthorised_vaccs = self.fhir_service.search_immunizations( - search_params.patient_identifier, - search_params.immunization_targets, - create_query_string(search_params), - supplier_system, - search_params.date_from, - search_params.date_to, - ) - except UnauthorizedVaxError as unauthorized: - return create_response(403, unauthorized.to_operation_outcome()) - - if "diagnostics" in result: - exp_error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.invariant, - diagnostics=result["diagnostics"], - ) - return create_response(400, json.dumps(exp_error)) - # Workaround for fhir.resources JSON removing the empty "entry" list. - result_json_dict: dict = json.loads(result.json()) - if "entry" in result_json_dict: - result_json_dict["entry"] = [ - entry - for entry in result_json_dict["entry"] - if entry["resource"].get("status") not in ("not-done", "entered-in-error") - ] - total_count = sum(1 for entry in result_json_dict["entry"] if entry.get("search", {}).get("mode") == "match") - result_json_dict["total"] = total_count - if request_contained_unauthorised_vaccs: - exp_error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.warning, - code=Code.unauthorized, - diagnostics="Your search contains details that you are not authorised to request", - ) - result_json_dict["entry"].append({"resource": exp_error}) - if "entry" not in result_json_dict: - result_json_dict["entry"] = [] - result_json_dict["total"] = 0 - return create_response(200, json.dumps(result_json_dict)) + return self._search_immunizations(parsed_search_params, supplier_system) + + def _get_immunization_by_identifier(self, search_params: dict[str, list[str]], supplier_system: str) -> dict: + raw_identifier, element = validate_and_retrieve_identifier_search_params(search_params) + identifier_components = raw_identifier.split("|", 1) + identifier = Identifier.construct(system=identifier_components[0], value=identifier_components[1]) + + search_bundle = self.fhir_service.get_immunization_by_identifier(identifier, supplier_system, element) + prepared_search_bundle = self._prepare_search_bundle(search_bundle) + + return create_response(200, prepared_search_bundle) + + def _search_immunizations(self, search_params: dict[str, list[str]], supplier_system: str) -> dict: + validated_search_params = validate_and_retrieve_search_params(search_params) + + search_bundle = self.fhir_service.search_immunizations( + validated_search_params.patient_identifier, + validated_search_params.immunization_targets, + supplier_system, + validated_search_params.date_from, + validated_search_params.date_to, + validated_search_params.include, + ) + + if self._has_too_many_search_results(search_bundle): + raise TooManyResultsError("Search returned too many results. Please narrow down the search") + + prepared_search_bundle = self._prepare_search_bundle(search_bundle) + + return create_response(200, prepared_search_bundle) def _is_valid_immunization_id(self, immunization_id: str) -> bool: """Validates if the given unique Immunization ID is valid.""" return False if not re.match(self._IMMUNIZATION_ID_PATTERN, immunization_id) else True + @staticmethod + def _prepare_search_bundle(search_response: Bundle) -> dict: + """Workaround for fhir.resources dict() or json() removing the empty "entry" list. Team also specified that + total should be the final key in the object. Should investigate if this can be resolved with later version of + the library.""" + search_response_dict = json.loads(search_response.json()) + + if "entry" not in search_response_dict: + search_response_dict["entry"] = [] + + search_response_dict["total"] = search_response_dict.pop("total") + return search_response_dict + @staticmethod def _is_valid_resource_version(resource_version: str) -> bool: return resource_version.isdigit() and int(resource_version) > 0 - def _validate_identifier_system(self, _id: str, _elements: str) -> Optional[dict]: - if not _id: - return create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.invalid, - diagnostics=( - "Search parameter identifier must have one value and must be in the format of " - '"identifier.system|identifier.value" ' - 'e.g. "http://xyz.org/vaccs|2345-gh3s-r53h7-12ny"' - ), - ) - if "|" not in _id or " " in _id: - return create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.invalid, - diagnostics=( - "Search parameter identifier must be in the format of " - '"identifier.system|identifier.value" ' - 'e.g. "http://xyz.org/vaccs|2345-gh3s-r53h7-12ny"' - ), - ) - - if not _elements: - return None - - requested_elements = {e.strip().lower() for e in _elements.split(",") if e.strip()} - requested_elements_valid = requested_elements.issubset({"id", "meta"}) - if _elements and not requested_elements_valid: - return create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.invalid, - diagnostics="_elements must be one or more of the following: id,meta", - ) - - def _create_bad_request(self, message): - error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.invalid, - diagnostics=message, + @staticmethod + def _is_identifier_search(search_params: dict[str, list[str]]) -> bool: + """Checks whether a given search is an identifier or patient + vacc type search based on the parameters""" + return ( + IdentifierSearchParameterName.IDENTIFIER in search_params + or IdentifierSearchParameterName.ELEMENTS in search_params ) - return create_response(400, error) - - def fetch_identifier_system_and_element(self, event: dict): - """ - Extracts `identifier` and `_elements` from an incoming FHIR search request. - - FHIR search supports two input formats: - 1. GET search: parameters appear in the query string (e.g. ?identifier=abc123&_elements=id,meta) - 2. POST search: parameters appear in the request body, form-encoded (e.g. identifier=abc123&_elements=id,meta) - - This function handles both cases, returning: - - The extracted identifier value - - The extracted _elements value - - Any validation check result for disallowed keys - - Booleans indicating whether identifier/_elements were present - """ - - query_params = event.get("queryStringParameters", {}) - body = event["body"] - not_required_keys = [ - "-date.from", - "-date.to", - "-immunization.target", - "_include", - "patient.identifier", - ] - - # Get Search Query Parameters - if query_params and not body: - query_string_has_immunization_identifier = "identifier" in query_params - query_string_has_element = "_elements" in query_params - identifier = query_params.get("identifier", "") - element = query_params.get("_elements", "") - query_check = check_keys_in_sources(event, not_required_keys) - - return ( - identifier, - element, - query_check, - query_string_has_immunization_identifier, - query_string_has_element, - ) - - # Post Search Identifier by body form - if body and not query_params: - decoded_body = base64.b64decode(body).decode("utf-8") - parsed_body = urllib.parse.parse_qs(decoded_body) - # Attempt to extract 'identifier' and '_elements' - converted_identifier = "" - converted_elements = "" - identifier = parsed_body.get("identifier", "") - if identifier: - converted_identifier = "".join(identifier) - _elements = parsed_body.get("_elements", "") - if _elements: - converted_elements = "".join(_elements) - body_has_identifier = "identifier" in parsed_body - body_has_immunization_elements = "_elements" in parsed_body - body_check = check_keys_in_sources(event, not_required_keys) - return ( - converted_identifier, - converted_elements, - body_check, - body_has_identifier, - body_has_immunization_elements, - ) - - def create_response_for_identifier(self, not_required, has_identifier, has_element): - if "patient.identifier" in not_required and has_identifier: - error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.server_error, - diagnostics="Search parameter should have either identifier or patient.identifier", - ) - return create_response(400, error) - - if not_required and has_element: - error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.server_error, - diagnostics="Search parameter _elements must have the following parameter: identifier", - ) - return create_response(400, error) @staticmethod - def _identify_supplier_system(aws_event): - supplier_system = aws_event["headers"]["SupplierSystem"] - if not supplier_system: - raise UnauthorizedError() - return supplier_system + def _has_too_many_search_results(search_response: Bundle) -> bool: + """Checks whether the response is too large - 6MB Lambda limit. Note: this condition should never happen as it + would require a very large number of vaccs for a single patient. It is also very rudimentary and all it does is + ensure we can raise and return a nice looking error. Consider using pagination as a more robust approach.""" + return len(search_response.json(use_decimal=True)) > MAX_RESPONSE_SIZE_BYTES + + @staticmethod + def _get_search_params_from_request( + aws_event: APIGatewayProxyEventV1, is_post_endpoint_req: bool + ) -> dict[str, list[str]]: + """Simple helper function to retrieve the search params from the relevant part of the AWS event, depending on + which search endpoint is being used""" + if not is_post_endpoint_req: + multi_value_params = get_multi_value_query_params(aws_event) + return multi_value_params if multi_value_params is not None else {} + + form_body = aws_event.get("body") + + if not form_body: + return {} + + decoded_body = base64.b64decode(form_body).decode("utf-8") + return parse_qs(decoded_body) diff --git a/lambdas/backend/src/controller/parameter_parser.py b/lambdas/backend/src/controller/parameter_parser.py new file mode 100644 index 0000000000..b33f44c912 --- /dev/null +++ b/lambdas/backend/src/controller/parameter_parser.py @@ -0,0 +1,239 @@ +import datetime +from dataclasses import dataclass +from typing import Optional + +from common.models.constants import Constants +from common.models.utils.generic_utils import nhs_number_mod11_check +from common.redis_client import get_redis_client +from controller.constants import IdentifierSearchElement, IdentifierSearchParameterName, ImmunizationSearchParameterName +from models.errors import ParameterExceptionError + +DUPLICATED_PARAMETERS_ERROR_MESSAGE = 'Parameters may not be duplicated. Use commas for "or".' +INVALID_IDENTIFIER_ERROR_MESSAGE = ( + 'Search parameter identifier must have one value and must be in the format of "iden' + 'tifier.system|identifier.value" "http://xyz.org/vaccs|2345-gh3s-r53h7-12ny"' +) +NO_PARAMETERS_ERROR_MESSAGE = ( + f"No parameter provided. Search using either {IdentifierSearchParameterName.IDENTIFIER} or " + f"{ImmunizationSearchParameterName.PATIENT_IDENTIFIER}" +) + +PATIENT_IDENTIFIER_SYSTEM = "https://fhir.nhs.uk/Id/nhs-number" + + +@dataclass +class SearchParams: + patient_identifier: str + immunization_targets: set[str] + date_from: Optional[datetime.date] + date_to: Optional[datetime.date] + include: Optional[str] + + def __repr__(self): + return str(self.__dict__) + + +def process_patient_identifier(identifier_params: dict[str, list[str]]) -> str: + """Validate and parse patient identifier parameter. + + :raises ParameterExceptionError: + """ + patient_identifiers = identifier_params.get(ImmunizationSearchParameterName.PATIENT_IDENTIFIER, []) + + if len(patient_identifiers) != 1: + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.PATIENT_IDENTIFIER} must have one value." + ) + + patient_identifier_parts = patient_identifiers[0].split("|") + identifier_system = patient_identifier_parts[0] + + if len(patient_identifier_parts) != 2 or identifier_system != PATIENT_IDENTIFIER_SYSTEM: + raise ParameterExceptionError( + "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 ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.PATIENT_IDENTIFIER} must be a valid NHS number." + ) + + return nhs_number + + +def process_immunization_target(imms_params: dict[str, list[str]]) -> set[str]: + """Validate and parse immunization target parameter. + + :raises ParameterExceptionError: + """ + vaccine_types = [ + vaccine_type + for vaccine_type in set(imms_params.get(ImmunizationSearchParameterName.IMMUNIZATION_TARGET, [])) + if vaccine_type is not None + ] + + if len(vaccine_types) < 1: + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.IMMUNIZATION_TARGET} must have one or more values." + ) + + valid_vaccine_types = get_redis_client().hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY) + if any(x not in valid_vaccine_types for x in vaccine_types): + raise ParameterExceptionError( + f"{ImmunizationSearchParameterName.IMMUNIZATION_TARGET} must be one or more of the following: " + f"{', '.join(valid_vaccine_types)}" + ) + + return set(vaccine_types) + + +def process_mandatory_params(params: dict[str, list[str]]) -> tuple[str, set[str]]: + """Validate mandatory params and return (patient_identifier, vaccine_types). + Raises ParameterExceptionError for any validation error. + """ + patient_identifier = process_patient_identifier(params) + vaccine_types = process_immunization_target(params) + + return patient_identifier, vaccine_types + + +def process_optional_params( + params: dict[str, list[str]], +) -> tuple[Optional[datetime.date], Optional[datetime.date], Optional[str]]: + """Parse optional params (date.from, date.to, _include). + Raises ParameterExceptionError for any validation error. + """ + include = None + date_from = None + date_to = None + + date_froms = params.get(ImmunizationSearchParameterName.DATE_FROM, []) + date_tos = params.get(ImmunizationSearchParameterName.DATE_TO, []) + includes = params.get(ImmunizationSearchParameterName.INCLUDE, []) + + if date_froms: + if len(date_froms) != 1: + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.DATE_FROM} may have one value at most." + ) + try: + date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date() + except ValueError: + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.DATE_FROM} must be in format: YYYY-MM-DD" + ) + + if date_tos: + if len(date_tos) != 1: + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.DATE_TO} may have one value at most." + ) + try: + date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date() + except ValueError: + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.DATE_TO} must be in format: YYYY-MM-DD" + ) + + if includes: + if includes[0].lower() != "immunization:patient": + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.INCLUDE} may only be " + f"'Immunization:patient' if provided." + ) + include = includes[0] + + return date_from, date_to, include + + +def validate_and_retrieve_search_params(params: dict[str, list[str]]) -> SearchParams: + """Validate and retrieve search parameters. + :raises ParameterExceptionError: + """ + patient_identifier, vaccine_types = process_mandatory_params(params) + date_from, date_to, include = process_optional_params(params) + + if date_from and date_to and date_from > date_to: + raise ParameterExceptionError( + f"Search parameter {ImmunizationSearchParameterName.DATE_FROM} must be before " + f"{ImmunizationSearchParameterName.DATE_TO}" + ) + + return SearchParams(patient_identifier, vaccine_types, date_from, date_to, include) + + +def parse_search_params(search_params_in_req: dict[str, list[str]]) -> dict[str, list[str]]: + """Ensures the search params provided in the event do not contain duplicated keys. Will split the parameters + provided by comma separators. Raises a ParameterExceptionError for duplicated keys. Existing business logic stipulated + that the API only accepts comma separated values rather than multi-value.""" + if any([len(values) > 1 for _, values in search_params_in_req.items()]): + raise ParameterExceptionError(DUPLICATED_PARAMETERS_ERROR_MESSAGE) + + parsed_params = {} + + for key, param_values in search_params_in_req.items(): + if len(param_values) == 0: + parsed_params[key] = [] + continue + + parsed_params[key] = [param.strip() for param_str in param_values for param in param_str.split(",")] + + if len(parsed_params) == 0: + raise ParameterExceptionError(message=NO_PARAMETERS_ERROR_MESSAGE) + + return parsed_params + + +def check_identifier_search_params_contain_no_incorrect_keys(search_params: dict[str, list[str]]) -> bool: + for patient_vacc_type_search_param in ImmunizationSearchParameterName: + if patient_vacc_type_search_param in search_params: + return False + + return True + + +def check_elements_valid(elements: list[str]) -> bool: + return set(elements).issubset({IdentifierSearchElement.ID, IdentifierSearchElement.META}) + + +def validate_and_retrieve_identifier_search_params(params: dict[str, list[str]]) -> tuple[str, Optional[set[str]]]: + contains_no_patient_vacc_type_params = check_identifier_search_params_contain_no_incorrect_keys(params) + + if not contains_no_patient_vacc_type_params: + if ( + ImmunizationSearchParameterName.PATIENT_IDENTIFIER in params + and IdentifierSearchParameterName.IDENTIFIER in params + ): + raise ParameterExceptionError("Search parameter should have either identifier or patient.identifier") + + raise ParameterExceptionError("Identifier search included patient.identifier search parameters") + + # Would we want to permit searching with multiple identifiers in future? + identifiers_list = params.get(IdentifierSearchParameterName.IDENTIFIER, []) + + if len(identifiers_list) > 1: + raise ParameterExceptionError(INVALID_IDENTIFIER_ERROR_MESSAGE) + + identifier = identifiers_list[0] if identifiers_list else None + elements = params.get(IdentifierSearchParameterName.ELEMENTS) + + if not identifier: + if elements is not None: + raise ParameterExceptionError("Search parameter _elements must have the following parameter: identifier") + + raise ParameterExceptionError(INVALID_IDENTIFIER_ERROR_MESSAGE) + + if "|" not in identifier or " " in identifier: + raise ParameterExceptionError(INVALID_IDENTIFIER_ERROR_MESSAGE) + + if not elements: + return identifier, None + + if not check_elements_valid(elements): + raise ParameterExceptionError("_elements must be one or more of the following: id,meta") + + return identifier, set(elements) diff --git a/lambdas/backend/src/models/errors.py b/lambdas/backend/src/models/errors.py index c53e50569b..0174baa2b8 100644 --- a/lambdas/backend/src/models/errors.py +++ b/lambdas/backend/src/models/errors.py @@ -90,6 +90,14 @@ class ParameterExceptionError(RuntimeError): def __str__(self): return self.message + def to_operation_outcome(self) -> dict: + return create_operation_outcome( + resource_id=str(uuid.uuid4()), + severity=Severity.error, + code=Code.invalid, + diagnostics=self.message, + ) + @dataclass class InvalidImmunizationIdError(ApiValidationError): @@ -120,6 +128,21 @@ def to_operation_outcome(self) -> dict: ) +@dataclass +class TooManyResultsError(ApiValidationError): + """Use this when there are too many results returned""" + + message: str + + def to_operation_outcome(self) -> dict: + return create_operation_outcome( + resource_id=str(uuid.uuid4()), + severity=Severity.error, + code=Code.invalid, + diagnostics=self.message, + ) + + @dataclass class InconsistentIdError(ApiValidationError): """Use this when the specified id in the message is inconsistent with the path diff --git a/lambdas/backend/src/parameter_parser.py b/lambdas/backend/src/parameter_parser.py deleted file mode 100644 index b4f33b6dc1..0000000000 --- a/lambdas/backend/src/parameter_parser.py +++ /dev/null @@ -1,222 +0,0 @@ -import base64 -import datetime -from dataclasses import dataclass -from typing import Optional -from urllib.parse import parse_qs, quote, urlencode - -from aws_lambda_typing.events import APIGatewayProxyEventV1 - -from common.models.constants import Constants -from common.models.utils.generic_utils import nhs_number_mod11_check -from common.redis_client import get_redis_client -from models.errors import ParameterExceptionError - -ERROR_MESSAGE_DUPLICATED_PARAMETERS = 'Parameters may not be duplicated. Use commas for "or".' - -ParamValue = list[str] -ParamContainer = dict[str, ParamValue] - -patient_identifier_system = "https://fhir.nhs.uk/Id/nhs-number" - -patient_identifier_key = "patient.identifier" -immunization_target_key = "-immunization.target" -date_from_key = "-date.from" -date_from_default = datetime.date(1900, 1, 1) -date_to_key = "-date.to" -date_to_default = datetime.date(9999, 12, 31) -include_key = "_include" - - -@dataclass -class SearchParams: - patient_identifier: str - immunization_targets: list[str] - date_from: Optional[datetime.date] - date_to: Optional[datetime.date] - include: Optional[str] - - 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 ParameterExceptionError(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 ParameterExceptionError( - "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 ParameterExceptionError("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 ParameterExceptionError(f"Search parameter {immunization_target_key} must have one or more values.") - - valid_vaccine_types = get_redis_client().hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY) - if any(x not in valid_vaccine_types for x in vaccine_types): - raise ParameterExceptionError( - 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 ParameterExceptionError("; ".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.""" - - def split_and_flatten(input: list[str]): - return [x.strip() for xs in input for x in xs.split(",")] - - def parse_multi_value_query_parameters( - multi_value_query_params: dict[str, list[str]], - ) -> ParamContainer: - if any(len(v) > 1 for k, v in multi_value_query_params.items()): - raise ParameterExceptionError(ERROR_MESSAGE_DUPLICATED_PARAMETERS) - params = [(k, split_and_flatten(v)) for k, v in multi_value_query_params.items()] - - return dict(params) - - def parse_body_params(aws_event: APIGatewayProxyEventV1) -> ParamContainer: - http_method = aws_event.get("httpMethod") - content_type = aws_event.get("headers", {}).get("Content-Type") - if http_method == "POST" and content_type == "application/x-www-form-urlencoded": - body = aws_event.get("body", "") or "" - decoded_body = base64.b64decode(body).decode("utf-8") - parsed_body = parse_qs(decoded_body) - - if any(len(v) > 1 for k, v in parsed_body.items()): - raise ParameterExceptionError(ERROR_MESSAGE_DUPLICATED_PARAMETERS) - items = {k: split_and_flatten(v) for k, v in parsed_body.items()} - return items - return {} - - query_params = parse_multi_value_query_parameters(aws_event.get("multiValueQueryStringParameters", {}) or {}) - body_params = parse_body_params(aws_event) - - if len(set(query_params.keys()) & set(body_params.keys())) > 0: - raise ParameterExceptionError(ERROR_MESSAGE_DUPLICATED_PARAMETERS) - - parsed_params = { - key: sorted(query_params.get(key, []) + body_params.get(key, [])) - for key in (query_params.keys() | body_params.keys()) - } - - return parsed_params - - -def create_query_string(search_params: SearchParams) -> str: - params = [ - ( - immunization_target_key, - ",".join(map(quote, search_params.immunization_targets)), - ), - ( - patient_identifier_key, - f"{patient_identifier_system}|{search_params.patient_identifier}", - ), - *( - [(date_from_key, search_params.date_from.isoformat())] - if search_params.date_from and search_params.date_from != date_from_default - else [] - ), - *( - [(date_to_key, search_params.date_to.isoformat())] - if search_params.date_to and search_params.date_to != date_to_default - else [] - ), - *([(include_key, search_params.include)] if search_params.include else []), - ] - search_params_qs = urlencode(sorted(params, key=lambda x: x[0]), safe=",") - return search_params_qs diff --git a/lambdas/backend/src/repository/fhir_repository.py b/lambdas/backend/src/repository/fhir_repository.py index 3710171295..f6a1c1a340 100644 --- a/lambdas/backend/src/repository/fhir_repository.py +++ b/lambdas/backend/src/repository/fhir_repository.py @@ -12,7 +12,6 @@ from fhir.resources.R4B.identifier import Identifier from fhir.resources.R4B.immunization import Immunization from mypy_boto3_dynamodb.service_resource import DynamoDBServiceResource, Table -from responses import logger from common.models.constants import Constants from common.models.errors import ResourceNotFoundError @@ -95,25 +94,30 @@ class ImmunizationRepository: def __init__(self, table: Table): self.table = table - def get_immunization_by_identifier(self, identifier_pk: str) -> tuple[Optional[dict], Optional[str]]: + def get_immunization_by_identifier( + self, identifier: Identifier + ) -> tuple[Optional[dict], Optional[ImmunizationRecordMetadata]]: response = self.table.query( IndexName="IdentifierGSI", - KeyConditionExpression=Key("IdentifierPK").eq(identifier_pk), + KeyConditionExpression=Key("IdentifierPK").eq(self._make_identifier_pk(identifier)), ) + item = response.get("Items") - if "Items" in response and len(response["Items"]) > 0: - item = response["Items"][0] - vaccine_type = self._vaccine_type(item["PatientSK"]) - resource = json.loads(item["Resource"]) - version = int(response["Items"][0]["Version"]) - return { - "resource": resource, - "id": resource.get("id"), - "version": version, - }, vaccine_type - else: + if not item: return None, None + item = response["Items"][0] + resource = json.loads(item.get("Resource", {})) + + # VED-915 - investigate and hopefully stop returning deleted items + imms_record_meta = ImmunizationRecordMetadata( + identifier, + int(item.get("Version")), + is_deleted=self._is_logically_deleted_item(item), + is_reinstated=self._is_reinstated_item(item), + ) + return resource, imms_record_meta + def get_immunization_resource_and_metadata_by_id( self, imms_id: str, include_deleted: bool = False ) -> tuple[Optional[dict], Optional[ImmunizationRecordMetadata]]: @@ -124,10 +128,8 @@ def get_immunization_resource_and_metadata_by_id( if not item: return None, None - deleted_at_attr = item.get("DeletedAt") - - is_deleted = deleted_at_attr is not None and deleted_at_attr != Constants.REINSTATED_RECORD_STATUS - is_reinstated = deleted_at_attr == Constants.REINSTATED_RECORD_STATUS + is_deleted = self._is_logically_deleted_item(item) + is_reinstated = self._is_reinstated_item(item) if is_deleted and not include_deleted: return None, None @@ -300,30 +302,28 @@ def delete_immunization(self, imms_id: str, supplier_system: str) -> None: else: raise error - def find_immunizations(self, patient_identifier: str, vaccine_types: set): - """it should find all of the specified patient's Immunization events for all of the specified vaccine_types""" + def find_immunizations(self, patient_identifier: str, vaccine_types: set) -> list[dict]: condition = Key("PatientPK").eq(_make_patient_pk(patient_identifier)) is_not_deleted = Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated") - raw_items = self.get_all_items(condition, is_not_deleted) + ieds_resources = self.get_all_items(condition, is_not_deleted) - if raw_items: - # Filter the response to contain only the requested vaccine types - items = [x for x in raw_items if x["PatientSK"].split("#")[0] in vaccine_types] + if not ieds_resources: + return [] - # Return a list of the FHIR immunization resource JSON items - final_resources = [ - { - **json.loads(item["Resource"]), - "meta": {"versionId": int(item.get("Version", 1))}, - } - for item in items - ] + # Filter the response to contain only the requested vaccine types + filtered_ieds_resources = [x for x in ieds_resources if self._vaccine_type(x["PatientSK"]) in vaccine_types] - return final_resources - else: - logger.warning("no items matched patient_identifier filter!") - return [] + # Return a list of the FHIR immunization resource JSON items + final_resources = [ + { + **json.loads(item["Resource"]), + "meta": {"versionId": int(item.get("Version", 1))}, + } + for item in filtered_ieds_resources + ] + + return final_resources def get_all_items(self, condition, is_not_deleted): """Query DynamoDB and paginate through all results.""" @@ -353,6 +353,19 @@ def get_all_items(self, condition, is_not_deleted): return all_items @staticmethod - def _vaccine_type(patientsk) -> str: - parsed = [str.strip(str.lower(s)) for s in patientsk.split("#")] + def _vaccine_type(patient_sk: str) -> str: + parsed = [str.strip(s) for s in patient_sk.split("#")] return parsed[0] + + @staticmethod + def _make_identifier_pk(identifier: Identifier) -> str: + return f"{identifier.system}#{identifier.value}" + + @staticmethod + def _is_logically_deleted_item(ieds_item: dict) -> bool: + deleted_at_attr = ieds_item.get("DeletedAt") + return deleted_at_attr is not None and deleted_at_attr != Constants.REINSTATED_RECORD_STATUS + + @staticmethod + def _is_reinstated_item(ieds_item: dict) -> bool: + return ieds_item.get("DeletedAt") == Constants.REINSTATED_RECORD_STATUS diff --git a/lambdas/backend/src/search_imms_handler.py b/lambdas/backend/src/search_imms_handler.py index 40a47bab38..34583d8ee1 100644 --- a/lambdas/backend/src/search_imms_handler.py +++ b/lambdas/backend/src/search_imms_handler.py @@ -1,80 +1,22 @@ import argparse -import base64 import json -import logging import pprint -import urllib.parse -import uuid -from aws_lambda_typing import context as context_ -from aws_lambda_typing import events +from aws_lambda_typing.context import Context +from aws_lambda_typing.events import APIGatewayProxyEventV1 -from constants import GENERIC_SERVER_ERROR_DIAGNOSTICS_MESSAGE, MAX_RESPONSE_SIZE_BYTES -from controller.aws_apig_response_utils import create_response +from controller.constants import SEARCH_IMMS_POST_PATH from controller.fhir_controller import FhirController, make_controller from log_structure import function_info -from models.errors import Code, Severity, create_operation_outcome - -logging.basicConfig(level="INFO") -logger = logging.getLogger() @function_info -def search_imms_handler(event: events.APIGatewayProxyEventV1, _context: context_): +def search_imms_handler(event: APIGatewayProxyEventV1, _context: Context): return search_imms(event, make_controller()) -def search_imms(event: events.APIGatewayProxyEventV1, controller: FhirController): - try: - query_params = event.get("queryStringParameters", {}) - body = event.get("body") - body_has_immunization_identifier = False - query_string_has_immunization_identifier = False - query_string_has_element = False - body_has_immunization_element = False - if not (query_params is None and body is None): - if query_params: - query_string_has_immunization_identifier = "identifier" in event.get("queryStringParameters", {}) - query_string_has_element = "_elements" in event.get("queryStringParameters", {}) - # Decode body from base64 - if body: - decoded_body = base64.b64decode(body).decode("utf-8") - # Parse the URL encoded body - parsed_body = urllib.parse.parse_qs(decoded_body) - - # Check for 'identifier' in body - body_has_immunization_identifier = "identifier" in parsed_body - body_has_immunization_element = "_elements" in parsed_body - if ( - query_string_has_immunization_identifier - or body_has_immunization_identifier - or query_string_has_element - or body_has_immunization_element - ): - return controller.get_immunization_by_identifier(event) - response = controller.search_immunizations(event) - - result_json = json.dumps(response) - result_size = len(result_json.encode("utf-8")) - - if result_size > MAX_RESPONSE_SIZE_BYTES: - exp_error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.invalid, - diagnostics="Search returned too many results. Please narrow down the search", - ) - return create_response(400, exp_error) - return response - except Exception: # pylint: disable = broad-exception-caught - logger.exception("Unhandled exception") - exp_error = create_operation_outcome( - resource_id=str(uuid.uuid4()), - severity=Severity.error, - code=Code.server_error, - diagnostics=GENERIC_SERVER_ERROR_DIAGNOSTICS_MESSAGE, - ) - return create_response(500, exp_error) +def search_imms(event: APIGatewayProxyEventV1, controller: FhirController): + return controller.search_immunizations(event, is_post_endpoint_req=event.get("path") == SEARCH_IMMS_POST_PATH) if __name__ == "__main__": @@ -112,7 +54,7 @@ def search_imms(event: events.APIGatewayProxyEventV1, controller: FhirController ) args = parser.parse_args() - event: events.APIGatewayProxyEventV1 = { + event: APIGatewayProxyEventV1 = { "multiValueQueryStringParameters": { "patient.identifier": [args.patient_identifier], "-immunization.target": [",".join(args.immunization_target)], diff --git a/lambdas/backend/src/service/fhir_service.py b/lambdas/backend/src/service/fhir_service.py index 075e9cb8f7..9fd6b33076 100644 --- a/lambdas/backend/src/service/fhir_service.py +++ b/lambdas/backend/src/service/fhir_service.py @@ -1,8 +1,9 @@ +import copy import datetime import logging import os import uuid -from typing import Optional, Union, cast +from typing import Optional, cast from uuid import uuid4 from fhir.resources.R4B.bundle import ( @@ -16,27 +17,34 @@ from fhir.resources.R4B.fhirtypes import Id from fhir.resources.R4B.identifier import Identifier from fhir.resources.R4B.immunization import Immunization +from fhir.resources.R4B.operationoutcome import OperationOutcome -import parameter_parser from authorisation.api_operation_code import ApiOperationCode from authorisation.authoriser import Authoriser +from common.models.constants import Constants from common.models.errors import ( + Code, CustomValidationError, IdentifierDuplicationError, MandatoryError, ResourceNotFoundError, + Severity, + create_operation_outcome, ) from common.models.fhir_immunization import ImmunizationValidator from common.models.utils.generic_utils import ( - form_json, get_contained_patient, get_occurrence_datetime, + make_search_bundle, ) from common.models.utils.validation_utils import ( get_vaccine_type, + validate_has_status, validate_identifiers_match, validate_resource_versions_match, ) +from controller.constants import IMMUNIZATION_TARGET_LEGACY_KEY_NAME, ImmunizationSearchParameterName +from controller.parameter_parser import PATIENT_IDENTIFIER_SYSTEM from filter import Filter from models.errors import UnauthorizedVaxError from repository.fhir_repository import ImmunizationRepository @@ -70,6 +78,10 @@ def get_service_url( class FhirService: + _DATA_MISSING_DATE_TIME_ERROR_MSG = ( + "Data quality issue - immunisation with ID %s was found containing no occurrenceDateTime" + ) + def __init__( self, imms_repo: ImmunizationRepository, @@ -81,25 +93,26 @@ def __init__( self.validator = validator def get_immunization_by_identifier( - self, identifier_pk: str, supplier_name: str, identifier: str, element: str - ) -> Optional[dict]: + self, identifier: Identifier, supplier_name: str, elements: Optional[set[str]] + ) -> FhirBundle: """ - Get an Immunization by its ID. Return None if not found. If the patient doesn't have an NHS number, - return the Immunization. + Get an Immunization by its ID. Returns a FHIR Bundle containing the search results. """ base_url = f"{get_service_url()}/Immunization" - imms_resp, vaccination_type = self.immunization_repo.get_immunization_by_identifier(identifier_pk) + resource, resource_metadata = self.immunization_repo.get_immunization_by_identifier(identifier) + + if not resource: + return make_search_bundle(resource, None, elements, identifier, base_url) - if not imms_resp: - return form_json(imms_resp, None, None, base_url) + vaccination_type = get_vaccine_type(resource) if not self.authoriser.authorise(supplier_name, ApiOperationCode.SEARCH, {vaccination_type}): raise UnauthorizedVaxError() patient_full_url = f"urn:uuid:{str(uuid4())}" - filtered_resource = Filter.search(imms_resp["resource"], patient_full_url) - imms_resp["resource"] = filtered_resource - return form_json(imms_resp, element, identifier, base_url) + filtered_resource = Filter.search(resource, patient_full_url) + + return make_search_bundle(filtered_resource, resource_metadata.resource_version, elements, identifier, base_url) def get_immunization_and_version_by_id(self, imms_id: str, supplier_system: str) -> tuple[Immunization, str]: """ @@ -195,8 +208,106 @@ def delete_immunization(self, imms_id: str, supplier_system: str) -> None: self.immunization_repo.delete_immunization(imms_id, supplier_system) - @staticmethod - def is_valid_date_from(immunization: dict, date_from: Union[datetime.date, None]): + def search_immunizations( + self, + nhs_number: str, + vaccine_types: set[str], + supplier_system: str, + date_from: Optional[datetime.date], + date_to: Optional[datetime.date], + include: Optional[str], + ) -> FhirBundle: + """ + Finds all instances of Immunization(s) for a specified patient for the given specified vaccine type(s). + 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. + """ + permitted_vacc_types = self.authoriser.filter_permitted_vacc_types( + supplier_system, ApiOperationCode.SEARCH, vaccine_types + ) + + # Only raise error if supplier's request had no permitted vaccinations + if not permitted_vacc_types: + raise UnauthorizedVaxError() + + # Have to retrieve first and then inspect resource to filter by date + all_resources = self.immunization_repo.find_immunizations(nhs_number, permitted_vacc_types) + filtered_resources = self._filter_search_results_by_date_and_status( + immunizations=all_resources, date_from=date_from, date_to=date_to, status=Constants.COMPLETED_STATUS + ) + + # Create the patient URN for the fullUrl field. + # NOTE: This UUID is assigned when a SEARCH request is received and used only for referencing the patient + # resource from immunisation resources within the bundle. The fullUrl value we are using is a urn (hence the + # FHIR key name of "fullUrl" is somewhat misleading) which cannot be used to locate any externally stored + # patient resource. This is as agreed with VDS team for backwards compatibility with Immunisation History API. + patient_full_url = f"urn:uuid:{str(uuid4())}" + + # Adjust immunization resources for the SEARCH response + processed_resources = [Filter.search(imms, patient_full_url) for imms in copy.deepcopy(filtered_resources)] + entries = [ + BundleEntry( + resource=Immunization.parse_obj(imms), + search=BundleEntrySearch(mode="match"), + fullUrl=f"{get_service_url()}/Immunization/{imms['id']}", + ) + for imms in processed_resources + ] + + # Add patient resource if there is at least one immunization resource + if len(processed_resources) > 0: + imms_patient_record = get_contained_patient(filtered_resources[-1]) + entries.append( + BundleEntry( + resource=self.process_patient_for_bundle(imms_patient_record), + search=BundleEntrySearch(mode="include"), + fullUrl=patient_full_url, + ) + ) + + if len(vaccine_types) != len(permitted_vacc_types): + # Include Operation Outcome error in response but still return the vaccs the client was authorised for + entries.append( + BundleEntry( + resource=OperationOutcome.construct( + **create_operation_outcome( + resource_id=str(uuid.uuid4()), + severity=Severity.warning, + code=Code.unauthorized, + diagnostics="Your search contains details that you are not authorised to request", + ) + ) + ) + ) + + return FhirBundle( + type="searchset", + entry=entries, + link=[ + BundleLink( + relation="self", + url=self.create_url_for_bundle_link(permitted_vacc_types, nhs_number, date_from, date_to, include), + ) + ], + total=len(processed_resources), + ) + + def _filter_search_results_by_date_and_status( + self, + immunizations: list[dict], + date_from: Optional[datetime.date], + date_to: Optional[datetime.date], + status: Optional[str], + ) -> list[dict]: + return [ + immunization + for immunization in immunizations + if self.is_valid_date_from(immunization, date_from) + and self.is_valid_date_to(immunization, date_to) + and validate_has_status(immunization, status) + ] + + def is_valid_date_from(self, immunization: dict, date_from: Optional[datetime.date]): """ Returns False if immunization occurrence is earlier than the date_from, or True otherwise (also returns True if date_from is None) @@ -205,13 +316,12 @@ def is_valid_date_from(immunization: dict, date_from: Union[datetime.date, None] return True if (occurrence_datetime := get_occurrence_datetime(immunization)) is None: - # TODO: Log error if no date. + logger.error(self._DATA_MISSING_DATE_TIME_ERROR_MSG, immunization.get("id")) return True return occurrence_datetime.date() >= date_from - @staticmethod - def is_valid_date_to(immunization: dict, date_to: Union[datetime.date, None]): + def is_valid_date_to(self, immunization: dict, date_to: Optional[datetime.date]): """ Returns False if immunization occurrence is later than the date_to, or True otherwise (also returns True if date_to is None) @@ -220,7 +330,7 @@ def is_valid_date_to(immunization: dict, date_to: Union[datetime.date, None]): return True if (occurrence_datetime := get_occurrence_datetime(immunization)) is None: - # TODO: Log error if no date. + logger.error(self._DATA_MISSING_DATE_TIME_ERROR_MSG, immunization.get("id")) return True return occurrence_datetime.date() <= date_to @@ -249,90 +359,35 @@ def process_patient_for_bundle(patient: dict): return new_patient @staticmethod - def create_url_for_bundle_link(params, vaccine_types): - """ - Updates the immunization.target parameter to include the given vaccine types and returns the url for the search - bundle. - """ - base_url = f"{get_service_url()}/Immunization" - - # Update the immunization.target parameter - new_immunization_target_param = f"immunization.target={','.join(vaccine_types)}" - parameters = "&".join( - [(new_immunization_target_param if x.startswith("-immunization.target=") else x) for x in params.split("&")] + def create_url_for_bundle_link( + immunization_targets: set[str], + patient_nhs_number: str, + date_from: Optional[datetime.date], + date_to: Optional[datetime.date], + include: Optional[str], + ) -> str: + """Creates url for the searchset Bundle Link. Existing logic always sorted the parameters in the given order + below, so this has been maintained""" + base_url = f"{get_service_url()}/Immunization?" + date_from_optional_part = ( + f"{ImmunizationSearchParameterName.DATE_FROM}={date_from.isoformat()}&" if date_from is not None else "" ) - - return f"{base_url}?{parameters}" - - def search_immunizations( - self, - nhs_number: str, - vaccine_types: list[str], - params: str, - supplier_system: str, - date_from: datetime.date = parameter_parser.date_from_default, - date_to: datetime.date = parameter_parser.date_to_default, - ) -> tuple[FhirBundle, bool]: - """ - Finds all instances of Immunization(s) for a specified patient which are for the specified vaccine type(s). - 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. - """ - permitted_vacc_types = self.authoriser.filter_permitted_vacc_types( - supplier_system, ApiOperationCode.SEARCH, set(vaccine_types) + date_to_optional_part = ( + f"{ImmunizationSearchParameterName.DATE_TO}={date_to.isoformat()}&" if date_to is not None else "" ) - # Only raise error if supplier's request had no permitted vaccinations - if not permitted_vacc_types: - raise UnauthorizedVaxError() - - # Obtain all resources which are for the requested nhs number and vaccine type(s) and within the date range - resources = [ - r - for r in self.immunization_repo.find_immunizations(nhs_number, permitted_vacc_types) - if self.is_valid_date_from(r, date_from) and self.is_valid_date_to(r, date_to) - ] - - # Create the patient URN for the fullUrl field. - # NOTE: This UUID is assigned when a SEARCH request is received and used only for referencing the patient - # resource from immunisation resources within the bundle. The fullUrl value we are using is a urn (hence the - # FHIR key name of "fullUrl" is somewhat misleading) which cannot be used to locate any externally stored - # patient resource. This is as agreed with VDS team for backwards compatibility with Immunisation History API. - patient_full_url = f"urn:uuid:{str(uuid4())}" - - imms_patient_record = get_contained_patient(resources[-1]) if resources else None - - # Filter and amend the immunization resources for the SEARCH response - resources_filtered_for_search = [Filter.search(imms, patient_full_url) for imms in resources] - - # Add bundle entries for each of the immunization resources - entries = [ - BundleEntry( - resource=Immunization.parse_obj(imms), - search=BundleEntrySearch(mode="match"), - fullUrl=f"{get_service_url()}/Immunization/{imms['id']}", - ) - for imms in resources_filtered_for_search - ] - - # Add patient resource if there is at least one immunization resource - if len(resources) > 0: - entries.append( - BundleEntry( - resource=self.process_patient_for_bundle(imms_patient_record), - search=BundleEntrySearch(mode="include"), - fullUrl=patient_full_url, - ) - ) - - # Create the bundle - fhir_bundle = FhirBundle(resourceType="Bundle", type="searchset", entry=entries) - fhir_bundle.link = [ - BundleLink( - relation="self", - url=self.create_url_for_bundle_link(params, permitted_vacc_types), - ) - ] - supplier_requested_unauthorised_vaccs = len(vaccine_types) != len(permitted_vacc_types) + # Temporarily maintaining this for backwards compatibility with imms history, but we should remove it + immunization_targets_part = f"{IMMUNIZATION_TARGET_LEGACY_KEY_NAME}={','.join(immunization_targets)}&" + optional_include_part = f"{ImmunizationSearchParameterName.INCLUDE}={include}&" if include is not None else "" + patient_identifier_part = ( + f"{ImmunizationSearchParameterName.PATIENT_IDENTIFIER}={PATIENT_IDENTIFIER_SYSTEM}|{patient_nhs_number}" + ) - return fhir_bundle, supplier_requested_unauthorised_vaccs + return ( + base_url + + date_from_optional_part + + date_to_optional_part + + immunization_targets_part + + optional_include_part + + patient_identifier_part + ) diff --git a/lambdas/backend/tests/controller/test_fhir_api_exception_handler.py b/lambdas/backend/tests/controller/test_fhir_api_exception_handler.py index b0b6718c65..bf65a4f506 100644 --- a/lambdas/backend/tests/controller/test_fhir_api_exception_handler.py +++ b/lambdas/backend/tests/controller/test_fhir_api_exception_handler.py @@ -15,6 +15,7 @@ InvalidJsonError, InvalidResourceVersionError, InvalidStoredDataError, + ParameterExceptionError, ResourceVersionNotProvidedError, UnauthorizedError, UnauthorizedVaxError, @@ -58,6 +59,7 @@ def test_exception_handler_handles_custom_exception_and_returns_fhir_response(se (InconsistentIdentifierError("Identifiers do not match"), 400, "invariant", "Identifiers do not match"), (InvalidJsonError("Invalid JSON provided"), 400, "invalid", "Invalid JSON provided"), (CustomValidationError("This field was invalid"), 400, "invariant", "This field was invalid"), + (ParameterExceptionError("Invalid parameter"), 400, "invalid", "Invalid parameter"), ( ResourceVersionNotProvidedError(resource_type="Immunization"), 400, diff --git a/lambdas/backend/tests/controller/test_fhir_controller.py b/lambdas/backend/tests/controller/test_fhir_controller.py index a445efe5e6..c80866bf9f 100644 --- a/lambdas/backend/tests/controller/test_fhir_controller.py +++ b/lambdas/backend/tests/controller/test_fhir_controller.py @@ -1,13 +1,15 @@ import base64 +import copy +import datetime import json import unittest import urllib import urllib.parse import uuid from unittest.mock import ANY, Mock, create_autospec, patch -from urllib.parse import urlencode -from fhir.resources.R4B.bundle import Bundle +from fhir.resources.R4B.bundle import Bundle, BundleEntry, BundleLink +from fhir.resources.R4B.identifier import Identifier from fhir.resources.R4B.immunization import Immunization from common.models.errors import ( @@ -16,15 +18,13 @@ ) from controller.aws_apig_response_utils import create_response from controller.fhir_controller import FhirController +from controller.parameter_parser import PATIENT_IDENTIFIER_SYSTEM from models.errors import ( - ParameterExceptionError, UnauthorizedVaxError, UnhandledResponseError, ) -from parameter_parser import patient_identifier_system, process_search_params from repository.fhir_repository import ImmunizationRepository from service.fhir_service import FhirService -from test_common.testing_utils.generic_utils import load_json_data from test_common.testing_utils.immunization_utils import create_covid_immunization @@ -34,7 +34,7 @@ class TestFhirControllerBase(unittest.TestCase): def setUp(self): super().setUp() self.mock_redis = Mock() - self.redis_getter_patcher = patch("parameter_parser.get_redis_client") + self.redis_getter_patcher = patch("controller.parameter_parser.get_redis_client") self.mock_redis_getter = self.redis_getter_patcher.start() self.logger_info_patcher = patch("logging.Logger.info") self.mock_logger_info = self.logger_info_patcher.start() @@ -75,6 +75,10 @@ def test_no_body_no_header(self): class TestFhirControllerGetImmunizationByIdentifier(unittest.TestCase): + test_local_identifier = Identifier.construct( + system="https://supplierABC/identifiers/vacc", value="f10b59b3-fc73-4616-99c9-9e882ab31184" + ) + def setUp(self): self.service = create_autospec(FhirService) self.controller = FhirController(self.service) @@ -82,640 +86,323 @@ def setUp(self): self.mock_logger_info = self.logger_info_patcher.start() def tearDown(self): - self.logger_info_patcher.stop() + patch.stopall() - def test_get_imms_by_identifer(self): - """it should return Immunization Id if it exists""" + def test_get_immunization_by_identifier_is_successful(self): + """it should return a searchset Bundle containing the immunization if it exists""" # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } + self.service.get_immunization_by_identifier.return_value = Bundle.construct( + entry=[BundleEntry.construct(resource=Immunization.construct(**{"id": "something"}))], + link=[BundleLink.construct(relation="self", url="a-url")], + type="searchset", + total=1, + ) lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id,meta", + "multiValueQueryStringParameters": { + "identifier": [f"{self.test_local_identifier.system}|{self.test_local_identifier.value}"], + "_elements": ["id,meta"], }, "body": None, } - identifier = lambda_event.get("queryStringParameters", {}).get("identifier") - _element = lambda_event.get("queryStringParameters", {}).get("_elements") - identifiers = identifier.replace("|", "#") # When - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) # Then - self.service.get_immunization_by_identifier.assert_called_once_with(identifiers, "test", identifier, _element) - + self.service.get_immunization_by_identifier.assert_called_once_with( + self.test_local_identifier, "test", {"meta", "id"} + ) self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["id"], "test") - - def test_get_imms_by_identifer_header_missing(self): - """it should return Immunization Id if it exists""" - # Given - lambda_event = { - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id,meta", - }, - "body": None, - } - response = self.controller.get_immunization_by_identifier(lambda_event) - - self.assertEqual(response["statusCode"], 403) - - def test_not_found_for_identifier(self): - """it should return not-found OperationOutcome if it doesn't exist""" - # Given - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "Bundle", - "type": "searchset", - "link": [ + self.assertEqual( + response["body"], + json.dumps( { - "relation": "self", - "url": "https://internal-dev.api.service.nhs.uk/immunisation-fhir-api-pr-224/Immunization?immunization.target=COVID&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7C1345678940", + "resourceType": "Bundle", + "type": "searchset", + "link": [{"relation": "self", "url": "a-url"}], + "entry": [{"resource": {"resourceType": "Immunization", "id": "something"}}], + "total": 1, } - ], - "entry": [], - "total": 0, + ), + ) + + def test_get_immunization_by_identifier_is_successful_when_using_post_endpoint(self): + """though not properly documented or really recommended, there is a /Immunization/_search POST endpoint which + can be used for performing searches""" + # Given + self.service.get_immunization_by_identifier.return_value = Bundle.construct( + entry=[BundleEntry.construct(resource=Immunization.construct(**{"id": "something"}))], + link=[BundleLink.construct(relation="self", url="a-url")], + type="searchset", + total=1, + ) + form_data = { + "identifier": f"{self.test_local_identifier.system}|{self.test_local_identifier.value}", + "_elements": "id,meta", } + url_encoded_test_str = urllib.parse.urlencode(form_data) lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id,meta", - "SupplierSystem": "test", - }, - "body": None, + "headers": {"Content-Type": "xxx-www-form-urlencoded", "SupplierSystem": "test"}, + "multiValueQueryStringParameters": {}, + "body": base64.b64encode(url_encoded_test_str.encode("utf-8")).decode("utf-8"), } - identifier = lambda_event.get("queryStringParameters", {}).get("identifier") - _element = lambda_event.get("queryStringParameters", {}).get("_elements") - imms = identifier.replace("|", "#") # When - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event, is_post_endpoint_req=True) # Then - self.service.get_immunization_by_identifier.assert_called_once_with(imms, "test", identifier, _element) - + self.service.get_immunization_by_identifier.assert_called_once_with( + self.test_local_identifier, "test", {"meta", "id"} + ) self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "Bundle") - self.assertEqual(body["entry"], []) - self.assertEqual(body["total"], 0) + self.assertEqual( + response["body"], + json.dumps( + { + "resourceType": "Bundle", + "type": "searchset", + "link": [{"relation": "self", "url": "a-url"}], + "entry": [{"resource": {"resourceType": "Immunization", "id": "something"}}], + "total": 1, + } + ), + ) - def test_get_imms_by_identifer_patient_identifier_and_element_present(self): - """it should return Immunization Id if it exists""" + def test_get_immunization_by_identifier_returns_no_entries_when_no_results(self): + """it should return a searchset Bundle containing with no entries if the immunization does not exist""" # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } + self.service.get_immunization_by_identifier.return_value = Bundle.construct( + entry=[], link=[BundleLink.construct(relation="self", url="a-url")], type="searchset", total=1 + ) lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "patient.identifier": "test", - "_elements": "id,meta", + "multiValueQueryStringParameters": { + "identifier": [f"{self.test_local_identifier.system}|{self.test_local_identifier.value}"], + "_elements": ["id,meta"], }, "body": None, } - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - # Then - self.service.get_immunization_by_identifier.assert_not_called() - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_get_imms_by_identifer_both_body_and_query_params_present(self): - """it should return Immunization Id if it exists""" - # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } - lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "patient.identifier": "test", - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id,meta", - }, - "body": "aW1tdW5pemF0aW9uLmlkZW50aWZpZXI9aHR0cHMlM0ElMkYlMkZzdXBwbGllckFCQyUyRmlkZW50aWZpZXJzJTJGdmFjYyU3Q2YxMGI1OWIzLWZjNzMtNDYxNi05OWM5LTllODgyYWIzMTE4NCZfZWxlbWVudD1pZCUyQ21ldGEmaWQ9cw==", - } # When - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) # Then - self.service.get_immunization_by_identifier.assert_not_called() - - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") + self.service.get_immunization_by_identifier.assert_called_once_with( + self.test_local_identifier, "test", {"meta", "id"} + ) + self.assertEqual(response["statusCode"], 200) + self.assertEqual( + response["body"], + json.dumps( + { + "resourceType": "Bundle", + "type": "searchset", + "link": [{"relation": "self", "url": "a-url"}], + "entry": [], + "total": 1, + } + ), + ) - def test_get_imms_by_identifer_both_identifier_present(self): - """it should return Immunization Id if it exists""" + def test_get_imms_by_identifier_returns_authorization_error_when_header_missing(self): + """it should return a 403 status error when the header is not provided""" # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "patient.identifier": "test", - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id,meta", + "multiValueQueryStringParameters": { + "identifier": ["https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184"], + "_elements": ["id,meta"], }, "body": None, } - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - # Then - self.service.get_immunization_by_identifier.assert_not_called() + response = self.controller.search_immunizations(lambda_event) - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") + self.assertEqual(response["statusCode"], 403) - def test_get_imms_by_identifer_invalid_element(self): - """it should return 400 as it contain invalid _element if it exists""" + def test_get_imms_by_identifier_returns_validation_error_when_no_params_provided(self): + """it should return a 400 status error if the client provides no search parameters""" # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id,meta,name", - }, + "multiValueQueryStringParameters": {}, "body": None, } - # When - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "No parameter provided. Search using either identifier or patient.identifier", + ) + self.service.get_immunization_by_identifier.assert_not_called() - def test_validate_immunization_identifier_is_empty(self): - """it should return 400 as identifierSystem is missing""" - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "OperationOutcome", - "id": "f6857e0e-40d0-4e5e-9e2f-463f87d357c6", - "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, - "issue": [ - { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/Codesystem/http-error-codes", - "code": "INVALID", - } - ] - }, - "diagnostics": "The provided identifiervalue is either missing or not in the expected format.", - } - ], - } + def test_get_imms_by_identifier_returns_validation_error_when_params_are_duplicated(self): + """it should return a 400 status error if the client provides the same parameter key multiple times""" + # Given lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": {"identifier": "", "_elements": "id"}, + "multiValueQueryStringParameters": { + "identifier": ["https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184"], + "_elements": ["id,meta", "another_one_oops_use_commas_instead"], + }, "body": None, } - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) - self.assertEqual(self.service.get_immunization_by_identifier.call_count, 0) self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + 'Parameters may not be duplicated. Use commas for "or".', + ) + self.service.get_immunization_by_identifier.assert_not_called() - def test_validate_immunization_identifier_in_invalid_format(self): - """it should return 400 as identifierSystem is missing""" - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "OperationOutcome", - "id": "f6857e0e-40d0-4e5e-9e2f-463f87d357c6", - "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, - "issue": [ - { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/Codesystem/http-error-codes", - "code": "INVALID", - } - ] - }, - "diagnostics": "identifier must be in the format of identifier.system|identifier.value e.g. http://pinnacle.org/vaccs|2345-gh3s-r53h7-12ny", - } - ], - } + def test_get_imms_by_identifier_returns_validation_error_when_patient_identifier_params_provided(self): + """it should return a 400 status error if the client also provides the patient.identifier parameter which is + used for the separate NHS Number + vaccination type(s) search""" + # Given lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vaccf10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id", + "multiValueQueryStringParameters": { + "identifier": ["https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184"], + "patient.identifier": ["system|12345"], + "_elements": ["id,meta"], }, "body": None, } - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) - self.assertEqual(self.service.get_immunization_by_identifier.call_count, 0) self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "Search parameter should have either identifier or patient.identifier", + ) + self.service.get_immunization_by_identifier.assert_not_called() - def test_validate_immunization_identifier_having_whitespace(self): - """it should return 400 as identifierSystem is missing""" - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "OperationOutcome", - "id": "f6857e0e-40d0-4e5e-9e2f-463f87d357c6", - "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, - "issue": [ - { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/Codesystem/http-error-codes", - "code": "INVALID", - } - ] - }, - "diagnostics": "The provided identifiervalue is either missing or not in the expected format.", - } - ], - } + def test_get_imms_by_identifier_returns_validation_error_when_unsupported_search_keys_provided(self): + """it should return a 400 status error if the client provides search keys associated with the patient + + vaccination type(s) search e.g. -date.from""" + # Given lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vacc | f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id", + "multiValueQueryStringParameters": { + "identifier": ["https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184"], + "-date.from": ["2025-01-01"], + "_elements": ["id,meta"], }, "body": None, } - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) - self.assertEqual(self.service.get_immunization_by_identifier.call_count, 0) self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "Identifier search included patient.identifier search parameters", + ) + self.service.get_immunization_by_identifier.assert_not_called() - def test_validate_imms_id_invalid_vaccinetype(self): - """it should validate lambda's Immunization id""" + def test_get_imms_by_identifier_returns_validation_error_when_multiple_identifiers_provided(self): + """it should return a 400 status error if the client provides more than one identifier search param""" # Given - self.service.get_immunization_by_identifier.side_effect = UnauthorizedVaxError() lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id", + "multiValueQueryStringParameters": { + "identifier": [ + "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184," + + "https://supplierABC/identifiers/vacc|additional-search" + ], + "_elements": ["id,meta"], }, "body": None, } - identifier = lambda_event.get("queryStringParameters", {}).get("identifier") - _element = lambda_event.get("queryStringParameters", {}).get("_elements") - identifiers = identifier.replace("|", "#") - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - - # Then - self.service.get_immunization_by_identifier.assert_called_once_with(identifiers, "test", identifier, _element) - - self.assertEqual(response["statusCode"], 403) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - self.assertEqual(body["issue"][0]["code"], "forbidden") - - -class TestFhirControllerGetImmunizationByIdentifierPost(unittest.TestCase): - def setUp(self): - self.service = create_autospec(FhirService) - self.controller = FhirController(self.service) - - def set_up_lambda_event(self, body): - """Helper to create and set up a lambda event with the given body""" - return { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aWRlbnRpZmllcj1odHRwcyUzQSUyRiUyRnN1cHBsaWVyQUJDJTJGaWRlbnRpZmllcnMlMkZ2YWNjJTdDZjEwYjU5YjMtZmM3My00NjE2LTk5YzktOWU4ODJhYjMxMTg0Jl9lbGVtZW50cz1pZCUyQ21ldGEmaWQ9cw==", - } - - def parse_lambda_body(self, lambda_event): - """Helper to parse and decode lambda event body""" - decoded_body = base64.b64decode(lambda_event["body"]).decode("utf-8") - parsed_body = urllib.parse.parse_qs(decoded_body) - immunization_identifier = parsed_body.get("identifier", "") - converted_identifier = "".join(immunization_identifier) - element = parsed_body.get("_elements", "") - converted_element = "".join(element) - identifiers = converted_identifier.replace("|", "#") - return identifiers, converted_identifier, converted_element - - def test_get_imms_by_identifier(self): - """It should return Immunization Id if it exists""" - # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } - body = "identifier=https://supplierABC/identifiers/vacc#f10b59b3-fc73-4616-99c9-9e882ab31184&_elements=id|meta" - lambda_event = self.set_up_lambda_event(body) - identifiers, converted_identifier, converted_element = self.parse_lambda_body(lambda_event) - - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - - # Then - self.service.get_immunization_by_identifier.assert_called_once_with( - identifiers, "test", converted_identifier, converted_element - ) - self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["id"], "test") - - def test_not_found_for_identifier(self): - """It should return not-found OperationOutcome if it doesn't exist""" - # Given - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "Bundle", - "type": "searchset", - "link": [ - { - "relation": "self", - "url": "https://internal-dev.api.service.nhs.uk/immunisation-fhir-api-pr-224/Immunization?immunization.target=COVID&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7C1345678940", - } - ], - "entry": [], - "total": 0, - } - body = "identifier=https://supplierABC/identifiers/vacc#f10b59b3-fc73-4616-99c9-9e882ab31184&_elements=id|meta" - lambda_event = self.set_up_lambda_event(body) - identifiers, converted_identifier, converted_element = self.parse_lambda_body(lambda_event) - - # When - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) - # Then - self.service.get_immunization_by_identifier.assert_called_once_with( - identifiers, "test", converted_identifier, converted_element + self.assertEqual(response["statusCode"], 400) + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "Search parameter identifier must have one value and must be in the format of " + '"identifier.system|identifier.value" "http://xyz.org/vaccs|2345-gh3s-r53h7-12ny"', ) - self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "Bundle") - self.assertEqual(body["entry"], []) - self.assertEqual(body["total"], 0) - - def test_get_imms_by_identifer_patient_identifier_and_element_present(self): - """it should return 400 as its having invalid request""" - # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } - lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "cGF0aWVudC5pZGVudGlmaWVyPWh0dHBzJTNBJTJGJTJGZmhpci5uaHMudWslMkZJZCUyRm5ocy1udW1iZXIlN0M5NjkzNjMyMTA5Jl9lbGVtZW50cz1pZCUyQ21ldGE=", - } - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - # Then self.service.get_immunization_by_identifier.assert_not_called() - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_get_imms_by_identifer_imms_identifier_and_element_not_present(self): - """it should return 400 as its having invalid request""" + def test_get_imms_by_identifier_returns_validation_error_when_no_identifier_provided_but_elements_present(self): + """it should return a 400 status error if the client provides no identifier""" # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aWRlbnRpZmllcj1odHRwcyUzQSUyRiUyRnN1cHBsaWVyQUJDJTJGaWRlbnRpZmllcnMlMkZ2YWNjJSAgN0NmMTBiNTliMy1mYzczLTQ2MTYtOTljOS05ZTg4MmFiMzExODQmX2VsZW1lbnRzPWlkJTJDbWV0YSZpZD1z", - } - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - # Then - self.service.get_immunization_by_identifier.assert_not_called() - - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_validate_immunization_element_is_empty(self): - """it should return 400 as element is missing""" - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "OperationOutcome", - "id": "f6857e0e-40d0-4e5e-9e2f-463f87d357c6", - "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, - "issue": [ - { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/Codesystem/http-error-codes", - "code": "INVALID", - } - ] - }, - "diagnostics": "The provided identifiervalue is either missing or not in the expected format.", - } - ], - } - lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aW1tdW5pemF0aW9uLmlkZW50aWZpZXI9aHR0cHMlM0ElMkYlMkZzdXBwbGllckFCQyUyRmlkZW50aWZpZXJzJTJGdmFjYyU3Q2YxMGI1OWIzLWZjNzMtNDYxNi05OWM5LTllODgyYWIzMTE4NCZfZWxlbWVudD0nJw==", + "multiValueQueryStringParameters": { + "identifier": [], + "_elements": ["id,meta"], + }, + "body": None, } - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) - self.assertEqual(self.service.get_immunization_by_identifier.call_count, 0) self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "Search parameter _elements must have the following parameter: identifier", + ) + self.service.get_immunization_by_identifier.assert_not_called() - def test_validate_immunization_identifier_is_invalid(self): - """it should return 400 as identifierSystem is invalid""" + def test_get_imms_by_identifier_returns_validation_error_when_identifier_invalid(self): + """it should return a 400 status error if the client provides an invalid identifier i.e. not in the format of + {system}|{identifier}""" # Given - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "OperationOutcome", - "id": "f6857e0e-40d0-4e5e-9e2f-463f87d357c6", - "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, - "issue": [ - { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/Codesystem/http-error-codes", - "code": "INVALID", - } - ] - }, - "diagnostics": "The provided identifiervalue is either missing or not in the expected format.", - } - ], - } lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aW1tdW5pemF0aW9uLmlkZW50aWZpZXI9aHR0cHMlM0ElMkYlMkZzdXBwbGllckFCQyUyRmlkZW50aWZpZXJzJTJGdmFjYzdDZjEwYjU5YjMtZmM3My00NjE2LTk5YzktOWU4ODJhYjMxMTg0Jl9lbGVtZW50PWlkJTJDbWV0YSZpZD1z", + "multiValueQueryStringParameters": { + "identifier": ["incorrect-format-identifier"], + }, + "body": None, } - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) - self.assertEqual(self.service.get_immunization_by_identifier.call_count, 0) self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") - - def test_get_imms_by_identifer_both_identifier_present(self): - """it should return 400 as its having invalid request""" - # Given - # Given - self.service.get_immunization_by_identifier.return_value = { - "id": "test", - "Version": 1, - } - lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "cGF0aWVudC5pZGVudGlmaWVyPWh0dHBzJTNBJTJGJTJGZmhpci5uaHMudWslMkZJZCUyRm5ocy1udW1iZXIlN0M5NjkzNjMyMTA5Ji1pbW11bml6YXRpb24udGFyZ2V0PUNPVklEMTkmX2luY2x1ZGU9SW1tdW5pemF0aW9uJTNBcGF0aWVudCZpZGVudGlmaWVyPWh0dHBzJTNBJTJGJTJGc3VwcGxpZXJBQkMlMkZpZGVudGlmaWVycyUyRnZhY2MlN0NmMTBiNTliMy1mYzczLTQ2MTYtOTljOS05ZTg4MmFiMzExODQmX2VsZW1lbnRzPWlkJTJDbWV0YSZpZD1z", - } - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - # Then + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "Search parameter identifier must have one value and must be in the format of " + '"identifier.system|identifier.value" "http://xyz.org/vaccs|2345-gh3s-r53h7-12ny"', + ) self.service.get_immunization_by_identifier.assert_not_called() - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_get_imms_by_identifer_invalid_element(self): - """it should return 400 as it contain invalid _element if it exists""" + def test_get_imms_by_identifier_returns_validation_error_when_elements_invalid(self): + """it should return a 400 status error if the client provides an _elements identifier i.e. not either meta, id + or both""" # Given lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aWRlbnRpZmllcj1odHRwcyUzQSUyRiUyRnN1cHBsaWVyQUJDJTJGaWRlbnRpZmllcnMlMkZ2YWNjJTdDZjEwYjU5YjMtZmM3My00NjE2LTk5YzktOWU4ODJhYjMxMTg0Jl9lbGVtZW50cz1pZCUyQ21ldGElMkNuYW1l", - } - # When - response = self.controller.get_immunization_by_identifier(lambda_event) - - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_validate_immunization_identifier_is_empty(self): - """it should return 400 as identifierSystem is missing""" - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "OperationOutcome", - "id": "f6857e0e-40d0-4e5e-9e2f-463f87d357c6", - "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, - "issue": [ - { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/Codesystem/http-error-codes", - "code": "INVALID", - } - ] - }, - "diagnostics": "identifier must be in the format of identifier.system|identifier.value e.g. http://pinnacle.org/vaccs|2345-gh3s-r53h7-12ny", - } - ], - } - lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aWRlbnRpZmllcj0mX2VsZW1lbnRzPWlkJTJDbWV0YQ==", - } - response = self.controller.get_immunization_by_identifier(lambda_event) - - self.assertEqual(self.service.get_immunization_by_identifier.call_count, 0) - self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") - - def test_validate_immunization_identifier_having_whitespace(self): - """it should return 400 as whitespace in id""" - self.service.get_immunization_by_identifier.return_value = { - "resourceType": "OperationOutcome", - "id": "f6857e0e-40d0-4e5e-9e2f-463f87d357c6", - "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, - "issue": [ - { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/Codesystem/http-error-codes", - "code": "INVALID", - } - ] - }, - "diagnostics": "The provided identifiervalue is either missing or not in the expected format.", - } - ], - } - lambda_event = { - "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aWRlbnRpZmllcj1odHRwcyUzQSUyRiUyRnN1cHBsaWVyQUJDJTJGaWRlbnRpZmllcnMlMkZ2YWNjJSAgN0NmMTBiNTliMy1mYzczLTQ2MTYtOTljOS05ZTg4MmFiMzExODQmX2VsZW1lbnRzPWlkJTJDbWV0YSZpZD1z", + "multiValueQueryStringParameters": { + "identifier": [f"{self.test_local_identifier.system}|{self.test_local_identifier.value}"], + "_elements": ["id,invalid"], + }, + "body": None, } - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) - self.assertEqual(self.service.get_immunization_by_identifier.call_count, 0) self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "_elements must be one or more of the following: id,meta", + ) + self.service.get_immunization_by_identifier.assert_not_called() - def test_validate_imms_id_invalid_vaccinetype(self): + def test_validate_imms_id_handles_exception_thrown_in_service_layer(self): """it should validate lambda's Immunization id""" # Given self.service.get_immunization_by_identifier.side_effect = UnauthorizedVaxError() lambda_event = { "headers": {"SupplierSystem": "test"}, - "queryStringParameters": None, - "body": "aWRlbnRpZmllcj1odHRwcyUzQSUyRiUyRnN1cHBsaWVyQUJDJTJGaWRlbnRpZmllcnMlMkZ2YWNjJTdDZjEwYjU5YjMtZmM3My00NjE2LTk5YzktOWU4ODJhYjMxMTg0Jl9lbGVtZW50cz1pZCUyQ21ldGEmaWQ9cw==", + "multiValueQueryStringParameters": { + "identifier": [f"{self.test_local_identifier.system}|{self.test_local_identifier.value}"], + "_elements": ["id,meta"], + }, + "body": None, } - decoded_body = base64.b64decode(lambda_event["body"]).decode("utf-8") - # Parse the URL encoded body - parsed_body = urllib.parse.parse_qs(decoded_body) - - immunization_identifier = parsed_body.get("identifier", "") - converted_identifier = "".join(immunization_identifier) - element = parsed_body.get("_elements", "") - converted_element = "".join(element) - identifiers = converted_identifier.replace("|", "#") # When - response = self.controller.get_immunization_by_identifier(lambda_event) + response = self.controller.search_immunizations(lambda_event) # Then self.service.get_immunization_by_identifier.assert_called_once_with( - identifiers, "test", converted_identifier, converted_element + self.test_local_identifier, "test", {"meta", "id"} ) self.assertEqual(response["statusCode"], 403) @@ -1249,50 +936,52 @@ def test_immunization_unhandled_error(self): class TestSearchImmunizations(TestFhirControllerBase): - MOCK_REDIS_V2D_HKEYS = { - "PERTUSSIS", - "RSV", - "3IN1", - "MMR", - "HPV", - "MMRV", - "PNEUMOCOCCAL", - "SHINGLES", + MOCK_REDIS_V2D_HKEYS = [ "COVID", "FLU", - "MENACWY", + ] + + patient_identifier_key = "patient.identifier" + immunization_target_key = "-immunization.target" + date_from_key = "-date.from" + date_to_key = "-date.to" + include_key = "_include" + nhs_number_valid_value = "9000000009" + patient_identifier_valid_value = f"{PATIENT_IDENTIFIER_SYSTEM}|{nhs_number_valid_value}" + test_lambda_event = { + "headers": { + "SupplierSystem": "test", + }, + "multiValueQueryStringParameters": { + immunization_target_key: ["COVID"], + patient_identifier_key: [patient_identifier_valid_value], + date_from_key: ["2000-01-01"], + date_to_key: ["2024-01-01"], + include_key: ["Immunization:patient"], + }, } def setUp(self): super().setUp() self.service = create_autospec(FhirService) self.controller = FhirController(self.service) - self.patient_identifier_key = "patient.identifier" - self.immunization_target_key = "-immunization.target" - self.date_from_key = "-date.from" - self.date_to_key = "-date.to" - self.nhs_number_valid_value = "9000000009" - self.patient_identifier_valid_value = f"{patient_identifier_system}|{self.nhs_number_valid_value}" self.mock_redis.hkeys.return_value = self.MOCK_REDIS_V2D_HKEYS self.mock_redis_getter.return_value = self.mock_redis - def test_get_search_immunizations(self): - """it should search based on patient_identifier and immunization_target""" - search_result = Bundle.construct() - self.service.search_immunizations.return_value = search_result, False + def tearDown(self): + patch.stopall() - vaccine_type = "COVID" - params = f"{self.immunization_target_key}={vaccine_type}&" + urllib.parse.urlencode( - [ - ( - f"{self.patient_identifier_key}", - f"{self.patient_identifier_valid_value}", - ) - ] + def test_search_immunizations_is_successful(self): + """it should search based on patient_identifier and immunization_target""" + self.service.search_immunizations.return_value = Bundle.construct( + entry=[BundleEntry.construct(resource=Immunization.construct(**{"id": "something"}))], + link=[BundleLink.construct(relation="self", url="patient-search-url")], + type="searchset", + total=1, ) + vaccine_type = "COVID" lambda_event = { "headers": { - "Content-Type": "application/x-www-form-urlencoded", "SupplierSystem": "test", }, "multiValueQueryStringParameters": { @@ -1306,359 +995,200 @@ def test_get_search_immunizations(self): # Then self.service.search_immunizations.assert_called_once_with( - self.nhs_number_valid_value, [vaccine_type], params, "test", ANY, ANY + self.nhs_number_valid_value, {vaccine_type}, "test", None, None, None ) self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "Bundle") - - def test_get_search_immunizations_vax_permission_check(self): - """it should return a 403 error if the service raises an unauthorized error""" - self.service.search_immunizations.side_effect = UnauthorizedVaxError() - - vaccine_type = "COVID" - lambda_event = { - "SupplierSystem": "test", - "multiValueQueryStringParameters": { - self.immunization_target_key: [vaccine_type], - self.patient_identifier_key: [self.patient_identifier_valid_value], - }, - } - - # When - response = self.controller.search_immunizations(lambda_event) - - # Then - self.assertEqual(response["statusCode"], 403) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_get_search_immunizations_for_unauthorized_vaccine_type_search( - self, - ): - """it should return 200 and contains warning operation outcome as the user is not having authorization for one of the vaccine type""" - search_result = load_json_data("sample_immunization_response _for _not_done_event.json") - bundle = Bundle.parse_obj(search_result) - self.service.search_immunizations.return_value = bundle, True - - vaccine_type = ["COVID", "FLU"] - vaccine_type = ",".join(vaccine_type) - - lambda_event = { - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "test", - }, - "multiValueQueryStringParameters": { - self.immunization_target_key: [vaccine_type], - self.patient_identifier_key: [self.patient_identifier_valid_value], - }, - } - - # When - response = self.controller.search_immunizations(lambda_event) - self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "Bundle") - # Check if any resource in entry has resourceType "OperationOutcome" - operation_outcome_present = any( - entry["resource"]["resourceType"] == "OperationOutcome" for entry in body.get("entry", []) - ) - self.assertTrue( - operation_outcome_present, - "OperationOutcome resource is not present in the response", + self.assertEqual( + response["body"], + json.dumps( + { + "resourceType": "Bundle", + "type": "searchset", + "link": [{"relation": "self", "url": "patient-search-url"}], + "entry": [{"resource": {"resourceType": "Immunization", "id": "something"}}], + "total": 1, + } + ), ) - def test_get_search_immunizations_for_unauthorized_vaccine_type_search_400(self): - """it should return 400 as the request has an invalid vaccine type""" - search_result = load_json_data("sample_immunization_response _for _not_done_event.json") - bundle = Bundle.parse_obj(search_result) - self.service.search_immunizations.return_value = bundle, False - - vaccine_type = "FLUE" - - lambda_event = { - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "test", - }, - "multiValueQueryStringParameters": { - self.immunization_target_key: [vaccine_type], - self.patient_identifier_key: [self.patient_identifier_valid_value], - }, - } - - # When - response = self.controller.search_immunizations(lambda_event) - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_post_search_immunizations(self): - """it should search based on patient_identifier and immunization_target""" - search_result = Bundle.construct() - self.service.search_immunizations.return_value = search_result, False - - vaccine_type = "COVID" - params = f"{self.immunization_target_key}={vaccine_type}&" + urllib.parse.urlencode( - [ - ( - f"{self.patient_identifier_key}", - f"{self.patient_identifier_valid_value}", - ) - ] + def test_search_immunizations_is_successful_when_using_the_post_endpoint(self): + """though not properly documented or really recommended, there is a /Immunization/_search POST endpoint which + can be used for performing patient and vacc type searches""" + # Given + self.service.search_immunizations.return_value = Bundle.construct( + entry=[BundleEntry.construct(resource=Immunization.construct(**{"id": "something"}))], + link=[BundleLink.construct(relation="self", url="patient-search-url")], + type="searchset", + total=1, ) - # Construct the application/x-www-form-urlencoded body - body = { - self.patient_identifier_key: self.patient_identifier_valid_value, + vaccine_type = "COVID" + form_data = { self.immunization_target_key: vaccine_type, + self.patient_identifier_key: self.patient_identifier_valid_value, } - encoded_body = urlencode(body) - # Base64 encode the body - base64_encoded_body = base64.b64encode(encoded_body.encode("utf-8")).decode("utf-8") - - # Construct the lambda event + url_encoded_test_str = urllib.parse.urlencode(form_data) lambda_event = { - "httpMethod": "POST", - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "Test", - }, - "body": base64_encoded_body, + "headers": {"Content-Type": "xxx-www-form-urlencoded", "SupplierSystem": "test"}, + "multiValueQueryStringParameters": {}, + "body": base64.b64encode(url_encoded_test_str.encode("utf-8")).decode("utf-8"), } + # When - response = self.controller.search_immunizations(lambda_event) + response = self.controller.search_immunizations(lambda_event, is_post_endpoint_req=True) + # Then self.service.search_immunizations.assert_called_once_with( - self.nhs_number_valid_value, [vaccine_type], params, "Test", ANY, ANY + self.nhs_number_valid_value, {vaccine_type}, "test", None, None, None ) self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "Bundle") - - def test_post_search_immunizations_for_unauthorized_vaccine_type_search(self): - """it should return 200 and contains warning operation outcome as the user is not having authorization for one of the vaccine type""" - search_result = load_json_data("sample_immunization_response _for _not_done_event.json") - bundle = Bundle.parse_obj(search_result) - self.service.search_immunizations.return_value = bundle, True - - vaccine_type = "COVID", "FLU" - vaccine_type = ",".join(vaccine_type) - # Construct the application/x-www-form-urlencoded body - body = { - self.patient_identifier_key: self.patient_identifier_valid_value, - self.immunization_target_key: vaccine_type, - } - encoded_body = urlencode(body) - # Base64 encode the body - base64_encoded_body = base64.b64encode(encoded_body.encode("utf-8")).decode("utf-8") - - # Construct the lambda event - lambda_event = { - "httpMethod": "POST", - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "Test", - }, - "body": base64_encoded_body, - } - # When - response = self.controller.search_immunizations(lambda_event) - self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "Bundle") - # Check if any resource in entry has resourceType "OperationOutcome" - operation_outcome_present = any( - entry["resource"]["resourceType"] == "OperationOutcome" for entry in body.get("entry", []) - ) - self.assertTrue( - operation_outcome_present, - "OperationOutcome resource is not present in the response", + self.assertEqual( + response["body"], + json.dumps( + { + "resourceType": "Bundle", + "type": "searchset", + "link": [{"relation": "self", "url": "patient-search-url"}], + "entry": [{"resource": {"resourceType": "Immunization", "id": "something"}}], + "total": 1, + } + ), ) - def test_post_search_immunizations_for_unauthorized_vaccine_type_search_400(self): - """it should return 400 as the request is having invalid vaccine type""" - search_result = load_json_data("sample_immunization_response _for _not_done_event.json") - bundle = Bundle.parse_obj(search_result) - self.service.search_immunizations.return_value = bundle, False - - vaccine_type = "FLUE" - - # Construct the application/x-www-form-urlencoded body - body = { - self.patient_identifier_key: self.patient_identifier_valid_value, - self.immunization_target_key: vaccine_type, - } - encoded_body = urlencode(body) - # Base64 encode the body - base64_encoded_body = base64.b64encode(encoded_body.encode("utf-8")).decode("utf-8") - - # Construct the lambda event + def test_search_immunizations_returns_a_validation_error_when_duplicate_params_provided(self): + """it should return a 400 invalid error if the client provides duplicated keys in the parameters""" lambda_event = { - "httpMethod": "POST", "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "VaccineTypePermissions": "flu:search", - }, - "body": base64_encoded_body, - } - # When - response = self.controller.search_immunizations(lambda_event) - self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_post_search_immunizations_for_unauthorized_vaccine_type_search_403(self): - """it should return 403 as the user doesnt have vaccinetype permission""" - self.service.search_immunizations.side_effect = UnauthorizedVaxError() - - vaccine_type = ["COVID", "FLU"] - vaccine_type = ",".join(vaccine_type) - - # Construct the application/x-www-form-urlencoded body - body = { - self.patient_identifier_key: self.patient_identifier_valid_value, - self.immunization_target_key: vaccine_type, - } - encoded_body = urlencode(body) - # Base64 encode the body - base64_encoded_body = base64.b64encode(encoded_body.encode("utf-8")).decode("utf-8") - - # Construct the lambda event - lambda_event = { - "httpMethod": "POST", - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "Test", - }, - "body": base64_encoded_body, - } - # When - response = self.controller.search_immunizations(lambda_event) - self.assertEqual(response["statusCode"], 403) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - @patch("controller.fhir_controller.process_search_params", wraps=process_search_params) - def test_uses_parameter_parser(self, process_search_params: Mock): - self.mock_redis.hkeys.return_value = self.MOCK_REDIS_V2D_HKEYS - self.mock_redis_getter.return_value = self.mock_redis - lambda_event = { - "multiValueQueryStringParameters": { - self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], - self.immunization_target_key: ["a-disease-type"], - } - } - - self.controller.search_immunizations(lambda_event) - - process_search_params.assert_called_once_with( - { - self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], - self.immunization_target_key: ["a-disease-type"], - } - ) - - @patch("controller.fhir_controller.process_search_params") - def test_search_immunizations_returns_400_on_ParameterException_from_parameter_parser( - self, process_search_params: Mock - ): - lambda_event = { - "multiValueQueryStringParameters": { - self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], - self.immunization_target_key: ["a-disease-type"], - } - } - - process_search_params.side_effect = ParameterExceptionError("Test") - response = self.controller.search_immunizations(lambda_event) - - # Then - self.assertEqual(response["statusCode"], 400) - outcome = json.loads(response["body"]) - self.assertEqual(outcome["resourceType"], "OperationOutcome") - - def test_search_immunizations_returns_400_on_passing_superseded_nhs_number(self): - """This method should return 400 as input parameter has superseded nhs number.""" - search_result = { - "diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists" - } - self.service.search_immunizations.return_value = search_result, False - - vaccine_type = "COVID" - lambda_event = { - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "Test", + "SupplierSystem": "test", }, "multiValueQueryStringParameters": { - self.immunization_target_key: [vaccine_type], - self.patient_identifier_key: [self.patient_identifier_valid_value], + self.immunization_target_key: ["COVID"], + self.patient_identifier_key: [self.patient_identifier_valid_value, "another-one"], }, } # When response = self.controller.search_immunizations(lambda_event) + # Then self.assertEqual(response["statusCode"], 400) - body = json.loads(response["body"]) - self.assertEqual(body["resourceType"], "OperationOutcome") - - def test_search_immunizations_returns_200_remove_vaccine_not_done(self): - """This method should return 200 but remove the data which has status as not done.""" - search_result = load_json_data("sample_immunization_response _for _not_done_event.json") - bundle = Bundle.parse_obj(search_result) - self.service.search_immunizations.return_value = bundle, False - vaccine_type = "COVID" - lambda_event = { - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "Test", - }, - "multiValueQueryStringParameters": { - self.immunization_target_key: [vaccine_type], - self.patient_identifier_key: [self.patient_identifier_valid_value], - }, - } - - # When - response = self.controller.search_immunizations(lambda_event) - - self.assertEqual(response["statusCode"], 200) - body = json.loads(response["body"]) - for entry in body.get("entry", []): - self.assertNotEqual(entry.get("resource", {}).get("status"), "not-done", "entered-in-error") - - def test_self_link_excludes_extraneous_params(self): - search_result = Bundle.construct() - self.service.search_immunizations.return_value = search_result, False - vaccine_type = "COVID" - params = f"{self.immunization_target_key}={vaccine_type}&" + urllib.parse.urlencode( - [ - ( - f"{self.patient_identifier_key}", - f"{self.patient_identifier_valid_value}", + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + 'Parameters may not be duplicated. Use commas for "or".', + ) + self.service.search_immunizations.assert_not_called() + + def test_search_immunizations_returns_a_validation_error_when_patient_identifier_invalid(self): + """it should return a 400 invalid error for multiple invalid patient identifier scenarios""" + test_cases = [ + ( + [f"{self.patient_identifier_valid_value},and-another-id"], + "Search parameter patient.identifier must have one value.", + ), + ( + ["https://not-fhir.nhs.uk/Id/nhs-number|9000000009"], + 'patient.identifier must be in the format of "https://fhir.nhs.uk/Id/nhs-number|{NHS number}" e.g. ' + '"https://fhir.nhs.uk/Id/nhs-number|9000000009"', + ), + ( + ["https://fhir.nhs.uk/Id/nhs-number|9000450007"], + "Search parameter patient.identifier must be a valid NHS number.", + ), + ] + + for test_patient_id, expected_error in test_cases: + with self.subTest(msg=expected_error): + test_lambda_event = copy.deepcopy(self.test_lambda_event) + test_lambda_event["multiValueQueryStringParameters"]["patient.identifier"] = test_patient_id + + # When + response = self.controller.search_immunizations(test_lambda_event) + + # Then + self.assertEqual(response["statusCode"], 400) + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + expected_error, + ) + self.service.search_immunizations.assert_not_called() + + def test_search_immunizations_returns_a_validation_error_when_immunization_target_invalid(self): + """it should return a 400 invalid error for multiple invalid -immunization.target scenarios""" + test_cases = [ + ([], "Search parameter -immunization.target must have one or more values."), + (["COVID,FLU,CHICKENS"], "-immunization.target must be one or more of the following: COVID, FLU"), + ] + + for test_patient_id, expected_error in test_cases: + with self.subTest(msg=expected_error): + test_lambda_event = copy.deepcopy(self.test_lambda_event) + test_lambda_event["multiValueQueryStringParameters"]["-immunization.target"] = test_patient_id + + # When + response = self.controller.search_immunizations(test_lambda_event) + + # Then + self.assertEqual(response["statusCode"], 400) + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + expected_error, ) - ] + self.service.search_immunizations.assert_not_called() + + def test_search_immunizations_returns_a_validation_error_when_optional_params_invalid(self): + """it should return a 400 invalid error for multiple invalid optional parameter scenarios""" + test_cases = [ + ("-date.to", ["2000-01-01,2000-10-10"], "Search parameter -date.to may have one value at most."), + ("-date.to", ["hello-world"], "Search parameter -date.to must be in format: YYYY-MM-DD"), + ("-date.from", ["2000-01-01,2000-10-10"], "Search parameter -date.from may have one value at most."), + ("-date.from", ["hello-world"], "Search parameter -date.from must be in format: YYYY-MM-DD"), + ( + "_include", + ["not-permitted-for-inclusion"], + "Search parameter _include may only be 'Immunization:patient' if provided.", + ), + ] + + for key_with_error, test_data, expected_error in test_cases: + with self.subTest(msg=expected_error): + test_lambda_event = copy.deepcopy(self.test_lambda_event) + test_lambda_event["multiValueQueryStringParameters"][key_with_error] = test_data + + # When + response = self.controller.search_immunizations(test_lambda_event) + + # Then + self.assertEqual(response["statusCode"], 400) + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + expected_error, + ) + self.service.search_immunizations.assert_not_called() + + @patch("controller.fhir_controller.MAX_RESPONSE_SIZE_BYTES", 5) + def test_search_immunizations_raises_error_if_too_many_results_found(self): + """it should return an error if there are too many results in the response for Lambda to handle. In reality, + highly unlikely. If a concern, pagination should be implemented.""" + self.service.search_immunizations.return_value = Bundle.construct( + entry=[BundleEntry.construct(resource=Immunization.construct(**{"id": "something"}))], + link=[BundleLink.construct(relation="self", url="patient-search-url")], + type="searchset", + total=1, ) - lambda_event = { - "multiValueQueryStringParameters": { - self.patient_identifier_key: [self.patient_identifier_valid_value], - self.immunization_target_key: [vaccine_type], - "b": ["b,a"], - "a": ["b,a"], - }, - "body": None, - "headers": { - "Content-Type": "application/x-www-form-urlencoded", - "SupplierSystem": "Test", - }, - "httpMethod": "POST", - } - - self.controller.search_immunizations(lambda_event) + # When + response = self.controller.search_immunizations(self.test_lambda_event) + # Then self.service.search_immunizations.assert_called_once_with( - self.nhs_number_valid_value, [vaccine_type], params, "Test", ANY, ANY + self.nhs_number_valid_value, + {"COVID"}, + "test", + datetime.date(2000, 1, 1), + datetime.date(2024, 1, 1), + "Immunization:patient", + ) + self.assertEqual(response["statusCode"], 400) + self.assertEqual( + json.loads(response["body"])["issue"][0]["diagnostics"], + "Search returned too many results. Please narrow down the search", ) diff --git a/lambdas/backend/tests/test_parameter_parser.py b/lambdas/backend/tests/controller/test_parameter_parser.py similarity index 58% rename from lambdas/backend/tests/test_parameter_parser.py rename to lambdas/backend/tests/controller/test_parameter_parser.py index 9dbb1c4f3c..5d86c6ae46 100644 --- a/lambdas/backend/tests/test_parameter_parser.py +++ b/lambdas/backend/tests/controller/test_parameter_parser.py @@ -1,18 +1,10 @@ -import base64 -import datetime import unittest from unittest.mock import Mock, create_autospec, patch -from models.errors import ParameterExceptionError -from parameter_parser import ( - SearchParams, - create_query_string, - date_from_key, - date_to_key, - include_key, - process_params, - process_search_params, +from controller.parameter_parser import ( + validate_and_retrieve_search_params, ) +from models.errors import ParameterExceptionError from service.fhir_service import FhirService @@ -23,82 +15,19 @@ def setUp(self): self.immunization_target_key = "-immunization.target" self.date_from_key = "-date.from" self.date_to_key = "-date.to" + self.include_key = "_include" self.logger_info_patcher = patch("logging.Logger.info") self.mock_logger_info = self.logger_info_patcher.start() self.mock_redis = Mock() - self.redis_getter_patcher = patch("parameter_parser.get_redis_client") + self.redis_getter_patcher = patch("controller.parameter_parser.get_redis_client") self.mock_redis_getter = self.redis_getter_patcher.start() def tearDown(self): patch.stopall() - def test_process_params_combines_content_and_query_string(self): - lambda_event = { - "multiValueQueryStringParameters": { - self.patient_identifier_key: ["a"], - }, - "body": base64.b64encode(f"{self.immunization_target_key}=b".encode("utf-8")), - "headers": {"Content-Type": "application/x-www-form-urlencoded"}, - "httpMethod": "POST", - } - - processed_params = process_params(lambda_event) - - expected = { - self.patient_identifier_key: ["a"], - self.immunization_target_key: ["b"], - } - - self.assertEqual(expected, processed_params) - - def test_process_params_is_sorted(self): - lambda_event = { - "multiValueQueryStringParameters": { - self.patient_identifier_key: ["b,a"], - }, - "body": base64.b64encode(f"{self.immunization_target_key}=b,a".encode("utf-8")), - "headers": {"Content-Type": "application/x-www-form-urlencoded"}, - "httpMethod": "POST", - } - processed_params = process_params(lambda_event) - - for v in processed_params.values(): - self.assertEqual(sorted(v), v) - - def test_process_params_does_not_process_body_on_get(self): - lambda_event = { - "multiValueQueryStringParameters": { - self.patient_identifier_key: ["b,a"], - }, - "body": base64.b64encode( - f"{self.immunization_target_key}=b&{self.immunization_target_key}=a".encode("utf-8") - ), - "headers": {"Content-Type": "application/x-www-form-urlencoded"}, - "httpMethod": "GET", - } - processed_params = process_params(lambda_event) - - self.assertEqual(processed_params, {self.patient_identifier_key: ["a", "b"]}) - - def test_process_params_does_not_allow_anded_params(self): - lambda_event = { - "multiValueQueryStringParameters": { - self.patient_identifier_key: ["a,b"], - self.immunization_target_key: ["a", "b"], - }, - "body": None, - "headers": {"Content-Type": "application/x-www-form-urlencoded"}, - "httpMethod": "POST", - } - - with self.assertRaises(ParameterExceptionError) as e: - process_params(lambda_event) - - self.assertEqual(str(e.exception), 'Parameters may not be duplicated. Use commas for "or".') - def test_process_search_params_checks_patient_identifier_format(self): with self.assertRaises(ParameterExceptionError) as e: - _ = process_search_params({self.patient_identifier_key: ["9000000009"]}) + _ = validate_and_retrieve_search_params({self.patient_identifier_key: ["9000000009"]}) self.assertEqual( str(e.exception), "patient.identifier must be in the format of " @@ -107,7 +36,7 @@ def test_process_search_params_checks_patient_identifier_format(self): ) self.mock_redis.hkeys.return_value = ["RSV"] self.mock_redis_getter.return_value = self.mock_redis - process_search_params( + validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV"], @@ -118,8 +47,9 @@ def test_process_search_params_whitelists_immunization_target(self): mock_redis_key = "RSV" self.mock_redis.hkeys.return_value = [mock_redis_key] self.mock_redis_getter.return_value = self.mock_redis + with self.assertRaises(ParameterExceptionError) as e: - process_search_params( + validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: [ @@ -130,15 +60,14 @@ def test_process_search_params_whitelists_immunization_target(self): } ) self.assertEqual( - str(e.exception), - f"immunization-target must be one or more of the following: {mock_redis_key}", - f"Unexpected exception message: {str(e.exception)}", + str(e.exception), f"-immunization.target must be one or more of the following: {mock_redis_key}" ) def test_process_search_params_immunization_target(self): - self.mock_redis.hkeys.return_value = ["RSV"] + mock_redis_key = "RSV" + self.mock_redis.hkeys.return_value = [mock_redis_key] self.mock_redis_getter.return_value = self.mock_redis - params = process_search_params( + params = validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV"], @@ -150,7 +79,7 @@ def test_process_search_params_immunization_target(self): def test_search_params_date_from_must_be_before_date_to(self): self.mock_redis.hkeys.return_value = ["RSV"] self.mock_redis_getter.return_value = self.mock_redis - params = process_search_params( + params = validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV"], @@ -161,7 +90,7 @@ def test_search_params_date_from_must_be_before_date_to(self): self.assertIsNotNone(params) - params = process_search_params( + params = validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV"], @@ -173,7 +102,7 @@ def test_search_params_date_from_must_be_before_date_to(self): self.assertIsNotNone(params) with self.assertRaises(ParameterExceptionError) as e: - _ = process_search_params( + _ = validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV"], @@ -184,14 +113,14 @@ def test_search_params_date_from_must_be_before_date_to(self): self.assertEqual( str(e.exception), - f"Search parameter {date_from_key} must be before {date_to_key}", + f"Search parameter {self.date_from_key} must be before {self.date_to_key}", ) def test_process_search_params_immunization_target_is_mandatory(self): self.mock_redis.hkeys.return_value = ["RSV"] self.mock_redis_getter.return_value = self.mock_redis with self.assertRaises(ParameterExceptionError) as e: - _ = process_search_params( + _ = validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], } @@ -203,7 +132,7 @@ def test_process_search_params_immunization_target_is_mandatory(self): def test_process_search_params_patient_identifier_is_mandatory(self): with self.assertRaises(ParameterExceptionError) as e: - _ = process_search_params( + _ = validate_and_retrieve_search_params( { self.immunization_target_key: ["a-disease-type"], } @@ -213,32 +142,6 @@ def test_process_search_params_patient_identifier_is_mandatory(self): "Search parameter patient.identifier must have one value.", ) - def test_create_query_string_with_all_params(self): - search_params = SearchParams("a", ["b"], datetime.date(1, 2, 3), datetime.date(4, 5, 6), "c") - query_string = create_query_string(search_params) - expected = ( - "-date.from=0001-02-03&-date.to=0004-05-06&-immunization.target=b" - "&_include=c&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7Ca" - ) - - self.assertEqual(expected, query_string) - - def test_create_query_string_with_minimal_params(self): - search_params = SearchParams("a", ["b"], None, None, None) - query_string = create_query_string(search_params) - expected = "-immunization.target=b&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7Ca" - - self.assertEqual(expected, query_string) - - def test_create_query_string_with_multiple_immunization_targets_comma_separated( - self, - ): - search_params = SearchParams("a", ["b", "c"], None, None, None) - query_string = create_query_string(search_params) - 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, ): @@ -246,7 +149,7 @@ def test_process_search_params_dedupes_immunization_targets_and_respects_include self.mock_redis.hkeys.return_value = ["RSV", "FLU"] self.mock_redis_getter.return_value = self.mock_redis - params = process_search_params( + params = validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV", "RSV", "FLU"], @@ -255,19 +158,19 @@ def test_process_search_params_dedupes_immunization_targets_and_respects_include ) # immunization targets should be deduped and preserve valid entries - self.assertIsInstance(params.immunization_targets, list) - self.assertCountEqual(params.immunization_targets, ["RSV", "FLU"]) + self.assertIsInstance(params.immunization_targets, set) + 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): + def test_process_search_params_raises_date_errors(self): """When multiple date-related errors occur they should be returned together.""" self.mock_redis.hkeys.return_value = ["RSV"] self.mock_redis_getter.return_value = self.mock_redis with self.assertRaises(ParameterExceptionError) as e: - process_search_params( + validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV"], @@ -276,10 +179,7 @@ def test_process_search_params_aggregates_date_errors(self): } ) - 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" - ) + expected = f"Search parameter {self.date_from_key} may have one value at most." self.assertEqual(str(e.exception), expected) def test_process_search_params_invalid_nhs_number_is_rejected(self): @@ -289,7 +189,7 @@ def test_process_search_params_invalid_nhs_number_is_rejected(self): self.mock_redis_getter.return_value = self.mock_redis with self.assertRaises(ParameterExceptionError) as e: - process_search_params( + validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|1234567890"], # invalid mod11 self.immunization_target_key: ["RSV"], @@ -307,7 +207,7 @@ def test_process_search_params_invalid_include_value_is_rejected(self): self.mock_redis_getter.return_value = self.mock_redis with self.assertRaises(ParameterExceptionError) as e: - process_search_params( + validate_and_retrieve_search_params( { self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"], self.immunization_target_key: ["RSV"], @@ -317,5 +217,5 @@ def test_process_search_params_invalid_include_value_is_rejected(self): self.assertEqual( str(e.exception), - f"Search parameter {include_key} may only be 'Immunization:patient' if provided.", + f"Search parameter {self.include_key} may only be 'Immunization:patient' if provided.", ) diff --git a/lambdas/backend/tests/repository/test_fhir_repository.py b/lambdas/backend/tests/repository/test_fhir_repository.py index 2258ea810a..03b95eb7ed 100644 --- a/lambdas/backend/tests/repository/test_fhir_repository.py +++ b/lambdas/backend/tests/repository/test_fhir_repository.py @@ -53,8 +53,8 @@ def tearDown(self): patch.stopall() def test_get_immunization_by_identifier(self): - """it should find an Immunization by id""" - imms_id = "a-id#an-id" + """it should find an Immunization by identifier""" + test_identifier = Identifier.construct(system="a-system", value="a-value") resource = dict() resource["Resource"] = {"foo": "bar", "id": "test"} self.table.query = MagicMock( @@ -69,26 +69,25 @@ def test_get_immunization_by_identifier(self): } ) - immunisation, immunisation_type = self.repository.get_immunization_by_identifier(imms_id) + immunisation, immunisation_resource_meta = self.repository.get_immunization_by_identifier(test_identifier) self.table.query.assert_called_once_with( IndexName="IdentifierGSI", - KeyConditionExpression=Key("IdentifierPK").eq(imms_id), + KeyConditionExpression=Key("IdentifierPK").eq("a-system#a-value"), ) - self.assertDictEqual(immunisation["resource"], resource["Resource"]) - self.assertEqual(immunisation["version"], 1) - self.assertEqual(immunisation["id"], "test") - self.assertEqual(immunisation_type, "covid") + self.assertDictEqual(immunisation, resource["Resource"]) + self.assertEqual(immunisation_resource_meta.resource_version, 1) + self.assertEqual(immunisation_resource_meta.is_deleted, False) def test_immunization_not_found(self): """it should return None if Immunization doesn't exist""" - imms_id = "non-existent-id" + test_identifier = Identifier.construct(system="a-system", value="a-value-not-exist") self.table.query = MagicMock(return_value={}) - immunisation, immunisation_type = self.repository.get_immunization_by_identifier(imms_id) + immunisation, immunisation_resource_meta = self.repository.get_immunization_by_identifier(test_identifier) self.assertIsNone(immunisation) - self.assertIsNone(immunisation_type) + self.assertIsNone(immunisation_resource_meta) def test_check_immunization_identifier_exists_returns_true(self): """it should return true when a record does exist with the given identifier""" @@ -587,43 +586,54 @@ def setUp(self): def tearDown(self): patch.stopall() - def test_find_immunizations(self): - """it should find events with patient_identifier""" + def test_find_immunizations_returns_empty_list_when_not_found(self): + """it should return an empty list when nothing is found for the given query""" nhs_number = "a-patient-id" dynamo_response = {"ResponseMetadata": {"HTTPStatusCode": 200}, "Items": []} self.table.query = MagicMock(return_value=dynamo_response) - condition = Key("PatientPK").eq(_make_patient_pk(nhs_number)) - # When - _ = self.repository.find_immunizations(nhs_number, vaccine_types={"COVID"}) + result = self.repository.find_immunizations(nhs_number, vaccine_types={"COVID"}) # Then self.table.query.assert_called_once_with( IndexName="PatientGSI", - KeyConditionExpression=condition, - FilterExpression=ANY, + KeyConditionExpression=Key("PatientPK").eq(_make_patient_pk(nhs_number)), + FilterExpression=Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated"), ) + self.assertEqual(result, []) - def test_exclude_deleted(self): - """it should exclude records with DeletedAt attribute""" - dynamo_response = {"ResponseMetadata": {"HTTPStatusCode": 200}, "Items": []} - self.table.query = MagicMock(return_value=dynamo_response) + def test_find_immunizations_returns_resources_including_meta(self): + """it should map Resource list into a list of Immunizations""" + imms1 = {"id": 1, "meta": {"versionId": 1}} + imms2 = {"id": 2, "meta": {"versionId": 1}} + items = [ + { + "Resource": json.dumps(imms1), + "PatientSK": "COVID#some_other_text", + "Version": "1", + }, + { + "Resource": json.dumps(imms2), + "PatientSK": "COVID#some_other_text", + "Version": "1", + }, + ] - is_ = Attr("DeletedAt").not_exists() | Attr("DeletedAt").eq("reinstated") + dynamo_response = {"ResponseMetadata": {"HTTPStatusCode": 200}, "Items": items} + self.table.query = MagicMock(return_value=dynamo_response) # When - _ = self.repository.find_immunizations("an-id", {"COVID"}) + results = self.repository.find_immunizations("an-id", {"COVID"}) # Then - self.table.query.assert_called_once_with( - IndexName="PatientGSI", KeyConditionExpression=ANY, FilterExpression=is_ - ) + self.assertListEqual(results, [imms1, imms2]) - def test_map_results_to_immunizations(self): - """it should map Resource list into a list of Immunizations""" + def test_find_immunizations_filters_any_vacc_types_not_in_the_request(self): + """it should filter out any returns vacc types that we not in the requested parameters""" imms1 = {"id": 1, "meta": {"versionId": 1}} - imms2 = {"id": 2, "meta": {"versionId": 1}} + imms2 = {"id": 2, "meta": {"versionId": 2}} + imms3 = {"id": 3, "meta": {"versionId": 4}} items = [ { "Resource": json.dumps(imms1), @@ -632,8 +642,13 @@ def test_map_results_to_immunizations(self): }, { "Resource": json.dumps(imms2), - "PatientSK": "COVID#some_other_text", - "Version": "1", + "PatientSK": "FLU#some_other_text", + "Version": "2", + }, + { + "Resource": json.dumps(imms3), + "PatientSK": "MMR#some_other_text", + "Version": "4", }, ] @@ -641,7 +656,7 @@ def test_map_results_to_immunizations(self): self.table.query = MagicMock(return_value=dynamo_response) # When - results = self.repository.find_immunizations("an-id", {"COVID"}) + results = self.repository.find_immunizations("an-id", {"COVID", "FLU"}) # Then self.assertListEqual(results, [imms1, imms2]) diff --git a/lambdas/backend/tests/service/test_fhir_service.py b/lambdas/backend/tests/service/test_fhir_service.py index c0eb4bb236..674c5fc43e 100644 --- a/lambdas/backend/tests/service/test_fhir_service.py +++ b/lambdas/backend/tests/service/test_fhir_service.py @@ -2,13 +2,10 @@ import json import os import unittest -import uuid from copy import deepcopy -from decimal import Decimal from unittest.mock import Mock, create_autospec, patch -from fhir.resources.R4B.bundle import Bundle as FhirBundle -from fhir.resources.R4B.bundle import BundleEntry +from fhir.resources.R4B.bundle import BundleLink from fhir.resources.R4B.identifier import Identifier from fhir.resources.R4B.immunization import Immunization @@ -33,6 +30,7 @@ create_covid_immunization_dict, create_covid_immunization_dict_no_id, ) +from test_common.testing_utils.values_for_tests import ValidValues # Constants for use within the tests NHS_NUMBER_USED_IN_SAMPLE_DATA = "9000000009" @@ -199,12 +197,17 @@ def test_unauthorised_error_raised_when_user_lacks_permissions(self): self.imms_repo.get_immunization_resource_and_metadata_by_id.assert_called_once_with(imms_id) -class TestGetImmunizationIdentifier(unittest.TestCase): +class TestGetImmunizationByIdentifier(TestFhirServiceBase): """Tests for FhirService.get_immunization_by_id""" MOCK_SUPPLIER_NAME = "TestSupplier" + test_identifier = Identifier.construct(system="some-system", value="some-value") + mock_resource_meta = ImmunizationRecordMetadata( + test_identifier, resource_version=1, is_deleted=False, is_reinstated=False + ) def setUp(self): + super().setUp() self.authoriser = create_autospec(Authoriser) self.imms_repo = create_autospec(ImmunizationRepository) self.validator = create_autospec(ImmunizationValidator) @@ -216,78 +219,104 @@ def tearDown(self): patch.stopall() def test_get_immunization_by_identifier(self): - """it should find an Immunization by id""" - imms_id = "an-id#an-id" - identifier = "test" - element = "id,mEta,DDD" - - mock_resource = create_covid_immunization_dict(identifier) + """it should find an Immunization by identifier""" + mock_resource = create_covid_immunization_dict("1234-some-id") + self.mock_redis.hget.return_value = "COVID" + self.mock_redis_getter.return_value = self.mock_redis self.authoriser.authorise.return_value = True - self.imms_repo.get_immunization_by_identifier.return_value = ( - { - "resource": mock_resource, - "id": imms_id, - "version": 1, - }, - "covid", - ) + self.imms_repo.get_immunization_by_identifier.return_value = mock_resource, self.mock_resource_meta # When - service_resp = self.fhir_service.get_immunization_by_identifier( - imms_id, self.MOCK_SUPPLIER_NAME, identifier, element - ) + result = self.fhir_service.get_immunization_by_identifier(self.test_identifier, self.MOCK_SUPPLIER_NAME, None) # Then - self.imms_repo.get_immunization_by_identifier.assert_called_once_with(imms_id) - self.authoriser.authorise.assert_called_once_with(self.MOCK_SUPPLIER_NAME, ApiOperationCode.SEARCH, {"covid"}) - self.assertEqual(service_resp["resourceType"], "Bundle") - self.assertEqual(service_resp.get("type"), "searchset") - self.assertIn("entry", service_resp) - self.assertEqual(len(service_resp["entry"]), 1) - self.assertIn("total", service_resp) - self.assertEqual(service_resp["total"], 1) - res = service_resp["entry"][0]["resource"] - self.assertEqual(res["resourceType"], "Immunization") - self.assertEqual(res["id"], imms_id) + self.imms_repo.get_immunization_by_identifier.assert_called_once_with(self.test_identifier) + self.authoriser.authorise.assert_called_once_with(self.MOCK_SUPPLIER_NAME, ApiOperationCode.SEARCH, {"COVID"}) + + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 1) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?identifier=some-" + "system|some-value", + ), + ) + + # Search function adds meta to the resource + mock_resource["meta"] = {"versionId": "1"} + self.assertEqual(result.entry[0].resource, Immunization.parse_obj(mock_resource)) def test_get_immunization_by_identifier_raises_error_when_not_authorised(self): - """it should find an Immunization by id""" - imms = "an-id#an-id" - identifier = "test" - element = "id,mEta,DDD" + """it should return an unauthorized error when the client does not have authorization for the vacc type""" + mock_resource = create_covid_immunization_dict("1234-some-id") + self.mock_redis.hget.return_value = "COVID" + self.mock_redis_getter.return_value = self.mock_redis self.authoriser.authorise.return_value = False - self.imms_repo.get_immunization_by_identifier.return_value = ( - { - "id": "foo", - "version": 1, - }, - "covid", - ) + self.imms_repo.get_immunization_by_identifier.return_value = mock_resource, self.mock_resource_meta with self.assertRaises(UnauthorizedVaxError): # When - self.fhir_service.get_immunization_by_identifier(imms, self.MOCK_SUPPLIER_NAME, identifier, element) + self.fhir_service.get_immunization_by_identifier(self.test_identifier, self.MOCK_SUPPLIER_NAME, None) # Then - self.imms_repo.get_immunization_by_identifier.assert_called_once_with(imms) - self.authoriser.authorise.assert_called_once_with(self.MOCK_SUPPLIER_NAME, ApiOperationCode.SEARCH, {"covid"}) + self.imms_repo.get_immunization_by_identifier.assert_called_once_with(self.test_identifier) + self.authoriser.authorise.assert_called_once_with(self.MOCK_SUPPLIER_NAME, ApiOperationCode.SEARCH, {"COVID"}) - def test_immunization_not_found(self): - """it should return None if Immunization doesn't exist""" - imms_id = "none" - identifier = "test" - element = "id" + def test_get_immunization_by_identifier_returns_empty_search_when_not_found(self): + """it should return an empty search bundle when the resource is not found""" self.imms_repo.get_immunization_by_identifier.return_value = None, None # When - act_imms = self.fhir_service.get_immunization_by_identifier( - imms_id, self.MOCK_SUPPLIER_NAME, identifier, element + result = self.fhir_service.get_immunization_by_identifier(self.test_identifier, self.MOCK_SUPPLIER_NAME, None) + + # Then + self.imms_repo.get_immunization_by_identifier.assert_called_once_with(self.test_identifier) + + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 0) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?identifier=some-" + "system|some-value", + ), + ) + self.assertEqual(result.entry, []) + + def test_get_immunization_by_identifier_when_elements_parameter_provided(self): + """it should return a subset of the resource when the _elements parameter is provided""" + mock_resource = create_covid_immunization_dict("1234-some-id") + self.mock_redis.hget.return_value = "COVID" + self.mock_redis_getter.return_value = self.mock_redis + self.authoriser.authorise.return_value = True + self.imms_repo.get_immunization_by_identifier.return_value = mock_resource, self.mock_resource_meta + + # When + result = self.fhir_service.get_immunization_by_identifier( + self.test_identifier, self.MOCK_SUPPLIER_NAME, {"id", "meta"} ) # Then - self.imms_repo.get_immunization_by_identifier.assert_called_once_with(imms_id) + self.imms_repo.get_immunization_by_identifier.assert_called_once_with(self.test_identifier) + self.authoriser.authorise.assert_called_once_with(self.MOCK_SUPPLIER_NAME, ApiOperationCode.SEARCH, {"COVID"}) - self.assertEqual(act_imms["entry"], []) + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 1) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?identifier=some-" + "system|some-value&_elements=id,meta", + ), + ) + self.assertEqual( + result.entry[0].resource, + Immunization.construct(**{"resourceType": "Immunization", "id": "1234-some-id", "meta": {"versionId": 1}}), + ) class TestCreateImmunization(TestFhirServiceBase): @@ -695,12 +724,13 @@ def test_delete_immunization_throws_authorisation_exception_if_does_not_have_req self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.DELETE, {"FLU"}) -class TestSearchImmunizations(unittest.TestCase): +class TestSearchImmunizations(TestFhirServiceBase): """Tests for FhirService.search_immunizations""" MOCK_SUPPLIER_SYSTEM_NAME = "Test" def setUp(self): + super().setUp() os.environ["IMMUNIZATION_ENV"] = "internal-dev" os.environ["IMMUNIZATION_BASE_PATH"] = "immunisation-fhir-api/FHIR/R4" self.authoriser = create_autospec(Authoriser) @@ -709,452 +739,212 @@ def setUp(self): self.fhir_service = FhirService(self.imms_repo, self.authoriser, self.validator) self.nhs_search_param = "patient.identifier" self.vaccine_type_search_param = "-immunization.target" - self.sample_patient_resource = load_json_data("bundle_patient_resource.json") - def test_vaccine_type_search(self): - """It should search for the correct vaccine type""" - nhs_number = VALID_NHS_NUMBER + @patch("service.fhir_service.uuid4", return_value="123456789-12") + def test_search_immunizations_returns_results_as_a_search_bundle(self, mock_uuid): + """it should return the retrieved immunization resources within a search bundle""" + mock_resource = create_covid_immunization_dict("1234-some-id") vaccine_type = "COVID" - params = f"{self.nhs_search_param}={nhs_number}&{self.vaccine_type_search_param}={vaccine_type}" - self.authoriser.filter_permitted_vacc_types.return_value = {vaccine_type} - self.imms_repo.find_immunizations.return_value = [] + self.imms_repo.find_immunizations.return_value = [mock_resource] # When - _ = self.fhir_service.search_immunizations(nhs_number, [vaccine_type], params, self.MOCK_SUPPLIER_SYSTEM_NAME) - - # Then - self.authoriser.filter_permitted_vacc_types.assert_called_once_with( - self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, {vaccine_type} - ) - self.imms_repo.find_immunizations.assert_called_once_with(nhs_number, {vaccine_type}) - - def test_make_fhir_bundle_from_search_result(self): - """It should return a FHIR Bundle resource""" - imms_ids = ["imms-1", "imms-2"] - imms_list = [create_covid_immunization_dict(imms_id) for imms_id in imms_ids] - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - nhs_number = NHS_NUMBER_USED_IN_SAMPLE_DATA - vaccine_types = ["COVID"] - params = f"{self.nhs_search_param}={nhs_number}&{self.vaccine_type_search_param}={vaccine_types}" - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - # When - result, contains_unauthorised_vaccs = self.fhir_service.search_immunizations( - nhs_number, vaccine_types, params, self.MOCK_SUPPLIER_SYSTEM_NAME + result = self.fhir_service.search_immunizations( + VALID_NHS_NUMBER, {vaccine_type}, self.MOCK_SUPPLIER_SYSTEM_NAME, None, None, None ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - # Then - self.assertIsInstance(result, FhirBundle) - self.assertEqual(result.type, "searchset") - self.assertEqual(len(imms_ids), len(searched_imms)) - self.assertFalse(contains_unauthorised_vaccs) - # Assert each entry in the bundle - for i, entry in enumerate(searched_imms): - self.assertIsInstance(entry, BundleEntry) - self.assertEqual(entry.resource.resource_type, "Immunization") - self.assertEqual(entry.resource.id, imms_ids[i]) - # Assert self link - self.assertEqual(len(result.link), 1) - self.assertEqual(result.link[0].relation, "self") - - def test_date_from_is_used_to_filter(self): - """It should return only Immunizations after date_from""" - # Arrange - imms = [ - ("imms-1", "2021-02-07T13:28:17.271+00:00"), - ("imms-2", "2021-02-08T13:28:17.271+00:00"), - ] - imms_list = [ - create_covid_immunization_dict(imms_id, occurrence_date_time=occcurrence_date_time) - for (imms_id, occcurrence_date_time) in imms - ] - imms_ids = [imms[0] for imms in imms] - nhs_number = NHS_NUMBER_USED_IN_SAMPLE_DATA - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - - # CASE: Day before. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_from=datetime.date(2021, 2, 6), - ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - self.assertEqual(2, len(searched_imms)) - for i, entry in enumerate(searched_imms): - self.assertEqual(imms_ids[i], entry.resource.id) - - # CASE:Day of first, inclusive search. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_from=datetime.date(2021, 2, 7), - ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] # Then - self.assertEqual(2, len(searched_imms)) - for i, entry in enumerate(searched_imms): - self.assertEqual(imms_ids[i], entry.resource.id) - - # CASE: Day of second, inclusive search. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_from=datetime.date(2021, 2, 8), - ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - self.assertEqual(1, len(searched_imms)) - self.assertEqual(imms_ids[1], searched_imms[0].resource.id) - - # CASE: Day after. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_from=datetime.date(2021, 2, 9), + self.imms_repo.find_immunizations.assert_called_once_with(VALID_NHS_NUMBER, {vaccine_type}) + mock_uuid.assert_called_once() + self.authoriser.filter_permitted_vacc_types.assert_called_once_with( + self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, {"COVID"} ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - self.assertEqual(0, len(searched_imms)) - - def test_date_from_is_optional(self): - """It should return everything when no date_from is specified""" - # Arrange - imms_ids = ["imms-1", "imms-2"] - imms_list = [create_covid_immunization_dict(imms_id) for imms_id in imms_ids] - nhs_number = NHS_NUMBER_USED_IN_SAMPLE_DATA - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - - # CASE: Without date_from - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations(nhs_number, vaccine_types, "", self.MOCK_SUPPLIER_SYSTEM_NAME) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - for i, entry in enumerate(searched_imms): - self.assertEqual(entry.resource.id, imms_ids[i]) - - # CASE: With date_from - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_from=datetime.date(2021, 3, 6), - ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 1) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?immunization-target" + "=COVID&patient.identifier=https://fhir.nhs.uk/Id/nhs-number|9990548609", + ), + ) + # Will contain the matched immunization and then the referenced patient resource + self.assertEqual(len(result.entry), 2) + self.assertEqual(result.entry[0].resource.json(), json.dumps(ValidValues.expected_resource_in_search)) + self.assertEqual(result.entry[-1].resource.resource_type, "Patient") + + @patch("service.fhir_service.uuid4", return_value="123456789-12") + def test_search_immunizations_filters_by_date_and_status(self, mock_uuid): + """it should only return the resources which occurred within the date filters and have a status of completed""" + mock_resource = create_covid_immunization_dict("1234-some-id", occurrence_date_time="2021-02-07T13:28:17+00:00") + mock_resource_filtered_date_from = create_covid_immunization_dict( + "1235-some-id", occurrence_date_time="2021-02-01T13:28:17+00:00" + ) + mock_resource_filtered_date_to = create_covid_immunization_dict( + "1236-some-id", occurrence_date_time="2023-02-07T13:28:17+00:00" + ) + mock_resource_filtered_status = create_covid_immunization_dict("1237-some-id", status="entered-in-error") - # Then - for i, entry in enumerate(searched_imms): - self.assertEqual(entry.resource.id, imms_ids[i]) - - def test_date_to_is_used_to_filter(self): - """It should return only Immunizations before date_to""" - # Arrange - imms = [ - ("imms-1", "2021-02-07T13:28:17.271+00:00"), - ("imms-2", "2021-02-08T13:28:17.271+00:00"), - ] - imms_list = [ - create_covid_immunization_dict(imms_id, occurrence_date_time=occcurrence_date_time) - for (imms_id, occcurrence_date_time) in imms + vaccine_type = "COVID" + self.authoriser.filter_permitted_vacc_types.return_value = {vaccine_type} + self.imms_repo.find_immunizations.return_value = [ + mock_resource_filtered_date_from, + mock_resource, + mock_resource_filtered_date_to, + mock_resource_filtered_status, ] - imms_ids = [imms[0] for imms in imms] - nhs_number = NHS_NUMBER_USED_IN_SAMPLE_DATA - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - - # CASE: Day after. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", + result = self.fhir_service.search_immunizations( + VALID_NHS_NUMBER, + {vaccine_type}, self.MOCK_SUPPLIER_SYSTEM_NAME, - date_to=datetime.date(2021, 2, 9), - ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - self.assertEqual(len(searched_imms), 2) - for i, entry in enumerate(searched_imms): - self.assertEqual(entry.resource.id, imms_ids[i]) - - # CASE: Day of second, inclusive search. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_to=datetime.date(2021, 2, 8), + datetime.date(2021, 2, 6), + datetime.date(2023, 1, 1), + None, ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] # Then - self.assertEqual(len(searched_imms), 2) - for i, entry in enumerate(searched_imms): - self.assertEqual(entry.resource.id, imms_ids[i]) - - # CASE: Day of first, inclusive search. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_to=datetime.date(2021, 2, 7), + self.imms_repo.find_immunizations.assert_called_once_with(VALID_NHS_NUMBER, {vaccine_type}) + mock_uuid.assert_called_once() + self.authoriser.filter_permitted_vacc_types.assert_called_once_with( + self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, {"COVID"} ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - # Then - self.assertEqual(len(searched_imms), 1) - self.assertEqual(searched_imms[0].resource.id, imms_ids[0]) - - # CASE: Day before. - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 1) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?-date.from=2021" + "-02-06&-date.to=2023-01-01&immunization-target=COVID&patient.identifier=" + "https://fhir.nhs.uk/Id/nhs-number|9990548609", + ), + ) + # Will contain the matched immunization and then the referenced patient resource + # And will filter out any additional resources + self.assertEqual(len(result.entry), 2) + self.assertEqual(result.entry[0].resource.json(), json.dumps(ValidValues.expected_resource_in_search)) + self.assertEqual(result.entry[-1].resource.resource_type, "Patient") + + @patch("service.fhir_service.uuid4", return_value="123456789-12") + def test_search_immunizations_adds_include_to_searched_url(self, mock_uuid): + """it should add the _include parameter into the returned url when the client provides it. Currently, it has no + effect on the resources returned as we always include the patient resource anyway""" + mock_resource = create_covid_immunization_dict("1234-some-id") + vaccine_type = "COVID" + self.authoriser.filter_permitted_vacc_types.return_value = {vaccine_type} + self.imms_repo.find_immunizations.return_value = [mock_resource] # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_to=datetime.date(2021, 2, 6), + result = self.fhir_service.search_immunizations( + VALID_NHS_NUMBER, {vaccine_type}, self.MOCK_SUPPLIER_SYSTEM_NAME, None, None, "Patient.identifier" ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - self.assertEqual(len(searched_imms), 0) - - def test_date_to_is_optional(self): - """It should return everything when no date_to is specified""" - # Arrange - imms_ids = ["imms-1", "imms-2"] - imms_list = [create_covid_immunization_dict(imms_id) for imms_id in imms_ids] - nhs_number = NHS_NUMBER_USED_IN_SAMPLE_DATA - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - - # CASE 1: Without date_to argument - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations(nhs_number, vaccine_types, "", self.MOCK_SUPPLIER_SYSTEM_NAME) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] # Then - for i, entry in enumerate(searched_imms): - self.assertEqual(entry.resource.id, imms_ids[i]) - - # CASE 2: With date_to argument - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) - - # When - result, _ = self.fhir_service.search_immunizations( - nhs_number, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, - date_to=datetime.date(2021, 3, 8), + self.imms_repo.find_immunizations.assert_called_once_with(VALID_NHS_NUMBER, {vaccine_type}) + mock_uuid.assert_called_once() + self.authoriser.filter_permitted_vacc_types.assert_called_once_with( + self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, {"COVID"} ) - searched_imms = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - for i, entry in enumerate(searched_imms): - self.assertEqual(entry.resource.id, imms_ids[i]) - - def test_immunization_resources_are_filtered_for_search(self): - """ - Test that each immunization resource returned is filtered to include only the appropriate fields for a search - response when the patient is Unrestricted - """ - # Arrange - imms_ids = ["imms-1", "imms-2"] - imms_list = [ - create_covid_immunization_dict( - imms_id, - NHS_NUMBER_USED_IN_SAMPLE_DATA, - occurrence_date_time="2021-02-07T13:28:17+00:00", - ) - for imms_id in imms_ids - ] - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - self.imms_repo.find_immunizations.return_value = deepcopy(imms_list) + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 1) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?immunization-target" + "=COVID&_include=Patient.identifier&patient.identifier=https://fhir.nhs.uk/Id/nhs-number|9990548609", + ), + ) + # Will contain the matched immunization and then the referenced patient resource + self.assertEqual(len(result.entry), 2) + self.assertEqual(result.entry[0].resource.json(), json.dumps(ValidValues.expected_resource_in_search)) + self.assertEqual(result.entry[-1].resource.resource_type, "Patient") + + def test_search_immunizations_returns_empty_bundle_when_no_results_found(self): + """it should return an empty search bundle when no results are found""" + vaccine_type = "FLU" + self.authoriser.filter_permitted_vacc_types.return_value = {vaccine_type} + self.imms_repo.find_immunizations.return_value = [] # When - result, _ = self.fhir_service.search_immunizations( - NHS_NUMBER_USED_IN_SAMPLE_DATA, - vaccine_types, - "", - self.MOCK_SUPPLIER_SYSTEM_NAME, + result = self.fhir_service.search_immunizations( + VALID_NHS_NUMBER, {vaccine_type}, self.MOCK_SUPPLIER_SYSTEM_NAME, None, None, None ) - searched_imms = [ - json.loads(entry.json(), parse_float=Decimal) - for entry in result.entry - if entry.resource.resource_type == "Immunization" - ] - searched_patient = [ - json.loads(entry.json()) for entry in result.entry if entry.resource.resource_type == "Patient" - ][0] # Then - expected_output_resource = load_json_data( - "completed_covid_immunization_event_filtered_for_search_using_bundle_patient_resource.json" + self.imms_repo.find_immunizations.assert_called_once_with(VALID_NHS_NUMBER, {vaccine_type}) + self.authoriser.filter_permitted_vacc_types.assert_called_once_with( + self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, {vaccine_type} ) - expected_output_resource["patient"]["reference"] = searched_patient["fullUrl"] - - for i, entry in enumerate(searched_imms): - # Check that entry has correct resource id - self.assertEqual(entry["resource"]["id"], imms_ids[i]) - - # Check that output is as expected (filtered, with id added) - expected_output_resource["id"] = imms_ids[i] - self.assertEqual(entry["resource"], expected_output_resource) - - def test_matches_contain_fullUrl(self): - """All matches must have a fullUrl consisting of their id. - See http://hl7.org/fhir/R4B/bundle-definitions.html#Bundle.entry.fullUrl. - Tested because fhir.resources validation doesn't check this as mandatory.""" - imms_ids = ["imms-1", "imms-2"] - imms_list = [create_covid_immunization_dict(imms_id) for imms_id in imms_ids] - self.imms_repo.find_immunizations.return_value = imms_list - nhs_number = NHS_NUMBER_USED_IN_SAMPLE_DATA - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - # When - result, _ = self.fhir_service.search_immunizations(nhs_number, vaccine_types, "", self.MOCK_SUPPLIER_SYSTEM_NAME) - entries = [entry for entry in result.entry if entry.resource.resource_type == "Immunization"] - - # Then - for i, entry in enumerate(entries): - self.assertEqual( - entry.fullUrl, - f"https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization/{imms_ids[i]}", - ) - - def test_patient_contains_fullUrl(self): - """Patient must have a fullUrl consisting of its id. - See http://hl7.org/fhir/R4B/bundle-definitions.html#Bundle.entry.fullUrl. - Tested because fhir.resources validation doesn't check this as mandatory.""" - imms_ids = ["imms-1", "imms-2"] - imms_list = [create_covid_immunization_dict(imms_id) for imms_id in imms_ids] - self.imms_repo.find_immunizations.return_value = imms_list - nhs_number = NHS_NUMBER_USED_IN_SAMPLE_DATA - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 0) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?immunization-target" + "=FLU&patient.identifier=https://fhir.nhs.uk/Id/nhs-number|9990548609", + ), + ) + self.assertEqual(len(result.entry), 0) + + @patch("service.fhir_service.uuid4", return_value="123456789-12") + def test_search_immunizations_includes_an_error_outcome_within_results_if_client_requests_unauthorised_vacc_types( + self, mock_uuid + ): + """it should return any vaccinations that the client is authorised to retrieve but also include an Operation + Outcome with an error if the requested anything they were not permitted to handle""" + mock_resource = create_covid_immunization_dict("1234-some-id") + vaccine_type = "COVID" + self.authoriser.filter_permitted_vacc_types.return_value = {vaccine_type} + self.imms_repo.find_immunizations.return_value = [mock_resource] # When - result, _ = self.fhir_service.search_immunizations(nhs_number, vaccine_types, "", self.MOCK_SUPPLIER_SYSTEM_NAME) - - # Then - patient_entry = next( - (entry for entry in result.entry if entry.resource.resource_type == "Patient"), - None, + result = self.fhir_service.search_immunizations( + VALID_NHS_NUMBER, {vaccine_type, "FLU"}, self.MOCK_SUPPLIER_SYSTEM_NAME, None, None, None ) - patient_full_url = patient_entry.fullUrl - self.assertTrue(patient_full_url.startswith("urn:uuid:")) - - # Check that final part of fullUrl is a uuid - patient_full_url_uuid = patient_full_url.split(":")[2] - self.assertTrue(uuid.UUID(patient_full_url_uuid)) - - def test_patient_included(self): - """Patient is included in the results.""" - - imms_ids = ["imms-1", "imms-2"] - imms_list = [create_covid_immunization_dict(imms_id) for imms_id in imms_ids] - self.imms_repo.find_immunizations.return_value = imms_list - nhs_number = VALID_NHS_NUMBER - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - - # When - result, _ = self.fhir_service.search_immunizations(nhs_number, vaccine_types, "", self.MOCK_SUPPLIER_SYSTEM_NAME) # Then - patient_entry = next((entry for entry in result.entry if entry.resource.resource_type == "Patient")) - self.assertIsNotNone(patient_entry) - - def test_patient_is_stripped(self): - """The included Patient is a subset of the data.""" - - imms_ids = ["imms-1", "imms-2"] - imms_list = [create_covid_immunization_dict(imms_id) for imms_id in imms_ids] - self.imms_repo.find_immunizations.return_value = imms_list - nhs_number = VALID_NHS_NUMBER - vaccine_types = ["COVID"] - self.authoriser.filter_permitted_vacc_types.return_value = set(vaccine_types) - - # When - result, _ = self.fhir_service.search_immunizations(nhs_number, vaccine_types, "", self.MOCK_SUPPLIER_SYSTEM_NAME) + # Does not pass FLU in as client is only permitted to retrieve COVID vaccinations + self.imms_repo.find_immunizations.assert_called_once_with(VALID_NHS_NUMBER, {vaccine_type}) + mock_uuid.assert_called_once() + self.authoriser.filter_permitted_vacc_types.assert_called_once_with( + self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, {"COVID", "FLU"} + ) - # Then - patient_entry = next((entry for entry in result.entry if entry.resource.resource_type == "Patient")) - patient_entry_resource = patient_entry.resource - fields_to_keep = ["id", "resource_type", "identifier"] - # self.assertListEqual(sorted(vars(patient_entry.resource).keys()), sorted(fields_to_keep)) - # self.assertGreater(len(patient), len(fields_to_keep)) - for field in fields_to_keep: - self.assertTrue(hasattr(patient_entry_resource, field), f"{field} in Patient") - self.assertIsNotNone(getattr(patient_entry_resource, field)) - - for k, v in vars(patient_entry_resource).items(): - if k not in fields_to_keep: - self.assertIsNone(v) + self.assertEqual(result.type, "searchset") + self.assertEqual(result.total, 1) + self.assertEqual( + result.link[0], + BundleLink.construct( + relation="self", + url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?immunization-target" + "=COVID&patient.identifier=https://fhir.nhs.uk/Id/nhs-number|9990548609", + ), + ) + # Will contain the matched immunization, the referenced patient resource and an OperationOutcome + self.assertEqual(len(result.entry), 3) + self.assertEqual(result.entry[0].resource.json(), json.dumps(ValidValues.expected_resource_in_search)) + self.assertEqual(result.entry[1].resource.resource_type, "Patient") + self.assertEqual(result.entry[2].resource.resource_type, "OperationOutcome") def test_search_raises_unauthorised_error_if_no_permissions(self): - """It should raise an UnauthorisedVaxError if the supplier does not have permissions for ANY of the requested + """it should raise an UnauthorisedVaxError if the supplier does not have permissions for ANY of the requested vaccination types""" vaccine_type = "COVID" - params = f"{self.nhs_search_param}={VALID_NHS_NUMBER}&{self.vaccine_type_search_param}={vaccine_type}" - self.authoriser.filter_permitted_vacc_types.return_value = {} # When with self.assertRaises(UnauthorizedVaxError): self.fhir_service.search_immunizations( - VALID_NHS_NUMBER, [vaccine_type], params, self.MOCK_SUPPLIER_SYSTEM_NAME + VALID_NHS_NUMBER, {vaccine_type}, self.MOCK_SUPPLIER_SYSTEM_NAME, None, None, None ) # Then @@ -1162,28 +952,3 @@ def test_search_raises_unauthorised_error_if_no_permissions(self): self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, {vaccine_type} ) self.imms_repo.find_immunizations.assert_not_called() - - def test_search_returns_successfully_but_flags_if_supplier_requests_vacc_types_without_perms( - self, - ): - """It should return a boolean indicating if the supplier has requested one or more vaccination types which - they do not have permission to search. This is a more permissive model and ensures that results are still - returned for the vacc types that they can handle""" - vaccine_types = ["COVID", "FLU"] - params = f"{self.nhs_search_param}={VALID_NHS_NUMBER}&{self.vaccine_type_search_param}={vaccine_types}" - - # Supplier only has permission for Covid - self.authoriser.filter_permitted_vacc_types.return_value = {"COVID"} - - # When - _, request_has_unauthorised_vacc_types = self.fhir_service.search_immunizations( - VALID_NHS_NUMBER, vaccine_types, params, self.MOCK_SUPPLIER_SYSTEM_NAME - ) - - # Then - self.authoriser.filter_permitted_vacc_types.assert_called_once_with( - self.MOCK_SUPPLIER_SYSTEM_NAME, ApiOperationCode.SEARCH, set(vaccine_types) - ) - # Only calls the repository for vacc types it is authorised for - self.imms_repo.find_immunizations.assert_called_once_with(VALID_NHS_NUMBER, {"COVID"}) - self.assertTrue(request_has_unauthorised_vacc_types) diff --git a/lambdas/backend/tests/test_search_imms.py b/lambdas/backend/tests/test_search_imms.py index 1ef035c875..20c1e61b2a 100644 --- a/lambdas/backend/tests/test_search_imms.py +++ b/lambdas/backend/tests/test_search_imms.py @@ -1,88 +1,43 @@ -import json import unittest -from pathlib import Path -from unittest.mock import create_autospec, patch +from unittest.mock import create_autospec -from constants import GENERIC_SERVER_ERROR_DIAGNOSTICS_MESSAGE from controller.fhir_controller import FhirController -from models.errors import Code, Severity, create_operation_outcome from search_imms_handler import search_imms -script_location = Path(__file__).absolute().parent - class TestSearchImmunizations(unittest.TestCase): def setUp(self): self.controller = create_autospec(FhirController) - self.logger_exception_patcher = patch("logging.Logger.exception") - self.mock_logger_exception = self.logger_exception_patcher.start() - self.logger_info_patcher = patch("logging.Logger.info") - self.mock_logger_info = self.logger_info_patcher.start() - self.logger_exception_patcher = patch("logging.Logger.exception") - self.mock_logger_exception = self.logger_exception_patcher.start() - - def tearDown(self): - patch.stopall() - - def test_search_immunizations(self): - """it should return a list of Immunizations""" - lambda_event = {"pathParameters": {"id": "an-id"}, "body": None} - exp_res = {"a-key": "a-value"} - - self.controller.search_immunizations.return_value = exp_res - - # When - act_res = search_imms(lambda_event, self.controller) - - # Then - self.controller.search_immunizations.assert_called_once_with(lambda_event) - self.assertDictEqual(exp_res, act_res) - def test_search_immunizations_to_get_imms_id(self): + def test_search_immunizations_calls_controller_with_correct_params_when_client_using_get_endpoint(self): """it should return a list of Immunizations""" lambda_event = { - "pathParameters": {"id": "an-id"}, - "queryStringParameters": { - "identifier": "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184", - "_elements": "id,meta", + "multiValueQueryStringParameters": { + "identifier": ["https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184"], + "_elements": ["id,meta"], }, + "path": "/Immunization", + "httpMethod": "GET", "body": None, } exp_res = {"a-key": "a-value"} - self.controller.get_immunization_by_identifier.return_value = exp_res + self.controller.search_immunizations.return_value = exp_res # When act_res = search_imms(lambda_event, self.controller) # Then - self.controller.get_immunization_by_identifier.assert_called_once_with(lambda_event) + self.controller.search_immunizations.assert_called_once_with(lambda_event, is_post_endpoint_req=False) self.assertDictEqual(exp_res, act_res) - def test_search_immunizations_get_id_from_body(self): + def test_search_immunizations_calls_controller_with_correct_params_when_client_using_post_endpoint(self): """it should return a list of Immunizations""" lambda_event = { - "pathParameters": {"id": "an-id"}, - "body": "cGF0aWVudC5pZGVudGlmaWVyPWh0dHBzJTNBJTJGJTJGZmhpci5uaHMudWslMkZJZCUyRm5ocy1udW1iZXIlN0M5NjkzNjMyMTA5Ji1pbW11bml6YXRpb24udGFyZ2V0PUNPVklEMTkmX2luY2x1ZGU9SW1tdW5pemF0aW9uJTNBcGF0aWVudCZpZGVudGlmaWVyPWh0dHBzJTNBJTJGJTJGc3VwcGxpZXJBQkMlMkZpZGVudGlmaWVycyUyRnZhY2MlN0NmMTBiNTliMy1mYzczLTQ2MTYtOTljOS05ZTg4MmFiMzExODQmX2VsZW1lbnRzPWlkJTJDbWV0YSZpZD1z", - "queryStringParameters": None, - } - exp_res = {"a-key": "a-value"} - - self.controller.get_immunization_by_identifier.return_value = exp_res - - # When - act_res = search_imms(lambda_event, self.controller) - - # Then - self.controller.get_immunization_by_identifier.assert_called_once_with(lambda_event) - self.assertDictEqual(exp_res, act_res) - - def test_search_immunizations_get_id_from_body_passing_none(self): - """it should enter search_immunizations as both the request params are none""" - lambda_event = { - "pathParameters": {"id": "an-id"}, - "body": None, - "queryStringParameters": None, + "multiValueQueryStringParameters": {}, + "path": "/Immunization/_search", + "httpMethod": "POST", + "body": "some+payload", } exp_res = {"a-key": "a-value"} @@ -92,78 +47,5 @@ def test_search_immunizations_get_id_from_body_passing_none(self): act_res = search_imms(lambda_event, self.controller) # Then - self.controller.search_immunizations.assert_called_once_with(lambda_event) + self.controller.search_immunizations.assert_called_once_with(lambda_event, is_post_endpoint_req=True) self.assertDictEqual(exp_res, act_res) - - def test_search_immunizations_get_id_from_body_element(self): - """it should enter into get_immunization_by_identifier only _element paramter is present""" - lambda_event = { - "pathParameters": {"id": "an-id"}, - "body": "X2VsZW1lbnRzPWlkJTJDbWV0YQ==", - "queryStringParameters": None, - } - exp_res = {"a-key": "a-value"} - - self.controller.get_immunization_by_identifier.return_value = exp_res - - # When - act_res = search_imms(lambda_event, self.controller) - - # Then - self.controller.get_immunization_by_identifier.assert_called_once_with(lambda_event) - self.assertDictEqual(exp_res, act_res) - - def test_search_immunizations_get_id_from_body_imms_identifer(self): - """it should enter into get_immunization_by_identifier only identifier paramter is present""" - lambda_event = { - "pathParameters": {"id": "an-id"}, - "body": "aWRlbnRpZmllcj1pZCUyQ21ldGE=", - "queryStringParameters": None, - } - exp_res = {"a-key": "a-value"} - - self.controller.get_immunization_by_identifier.return_value = exp_res - - # When - act_res = search_imms(lambda_event, self.controller) - - # Then - self.controller.get_immunization_by_identifier.assert_called_once_with(lambda_event) - self.assertDictEqual(exp_res, act_res) - - @patch("search_imms_handler.MAX_RESPONSE_SIZE_BYTES", 10) - def test_search_immunizations_lambda_size_limit(self): - """it should return 400 as search returned too many results.""" - lambda_event = {"pathParameters": {"id": "an-id"}, "body": None} - - self.controller.search_immunizations.return_value = {"response": "size is larger than lambda limit"} - - # When - act_res = search_imms(lambda_event, self.controller) - - # Then - self.controller.search_immunizations.assert_called_once_with(lambda_event) - self.assertEqual(act_res["statusCode"], 400) - - def test_search_handle_exception(self): - """unhandled exceptions should result in 500""" - lambda_event = {"pathParameters": {"id": "an-id"}} - error_msg = "an unhandled error" - self.controller.search_immunizations.side_effect = Exception(error_msg) - - exp_error = create_operation_outcome( - resource_id=None, - severity=Severity.error, - code=Code.server_error, - diagnostics=GENERIC_SERVER_ERROR_DIAGNOSTICS_MESSAGE, - ) - - # When - act_res = search_imms(lambda_event, self.controller) - - # Then - act_body = json.loads(act_res["body"]) - - self.assertEqual(exp_error["issue"][0]["code"], act_body["issue"][0]["code"]) - self.assertEqual(exp_error["issue"][0]["severity"], act_body["issue"][0]["severity"]) - self.assertEqual(act_res["statusCode"], 500) diff --git a/lambdas/shared/src/common/models/constants.py b/lambdas/shared/src/common/models/constants.py index b985ce126f..7bbe99c5ea 100644 --- a/lambdas/shared/src/common/models/constants.py +++ b/lambdas/shared/src/common/models/constants.py @@ -57,6 +57,7 @@ class Constants: VACCINE_TYPE_TO_DISEASES_HASH_KEY = "vacc_to_diseases" DISEASES_TO_VACCINE_TYPE_HASH_KEY = "diseases_to_vacc" + COMPLETED_STATUS = "completed" REINSTATED_RECORD_STATUS = "reinstated" diff --git a/lambdas/shared/src/common/models/utils/generic_utils.py b/lambdas/shared/src/common/models/utils/generic_utils.py index 8bd6fd9512..f8996fe66f 100644 --- a/lambdas/shared/src/common/models/utils/generic_utils.py +++ b/lambdas/shared/src/common/models/utils/generic_utils.py @@ -1,10 +1,10 @@ """Generic utilities""" import base64 +import copy import datetime -import json import urllib.parse -from typing import Any, Dict, Literal, Optional +from typing import Any, Literal, Optional from fhir.resources.R4B.bundle import ( Bundle as FhirBundle, @@ -14,6 +14,7 @@ BundleEntrySearch, BundleLink, ) +from fhir.resources.R4B.identifier import Identifier from fhir.resources.R4B.immunization import Immunization from stdnum.verhoeff import validate @@ -154,57 +155,55 @@ def create_diagnostics_error(value): return exp_error -def make_empty_bundle(self_url: str) -> Dict[str, Any]: - return { - "resourceType": "Bundle", - "type": "searchset", - "link": [{"relation": "self", "url": self_url}], - "entry": [], - "total": 0, - } +def make_empty_search_bundle(searched_url: str) -> FhirBundle: + return FhirBundle(entry=[], link=[BundleLink(relation="self", url=searched_url)], type="searchset", total=0) -def form_json(response, _elements, identifier, baseurl): - self_url = f"{baseurl}?identifier={identifier}" + (f"&_elements={_elements}" if _elements else "") +# A lot of stuff in here is not very generic. Consider moving to relevant lambdas +def make_search_bundle( + resource: Optional[dict], + version_id: Optional[int], + elements: Optional[set[str]], + identifier: Identifier, + base_url: str, +) -> FhirBundle: + searched_url = f"{base_url}?identifier={identifier.system}|{identifier.value}" + ( + f"&_elements={','.join(sorted(elements))}" if elements else "" + ) - if not response: - return make_empty_bundle(self_url) + if not resource: + return make_empty_search_bundle(searched_url) - meta = {"versionId": response["version"]} if "version" in response else {} + meta = {"versionId": version_id} - # Full Immunization payload to be returned if only the identifier parameter was provided and truncated - # when _elements is used - if _elements: - elements = {e.strip().lower() for e in _elements.split(",") if e.strip()} - resource = {"resourceType": "Immunization"} + # Full Immunization payload to be returned if only the identifier parameter was provided and truncated when + # _elements is used + if elements is not None: + resource_for_bundle: dict[str, Any] = {"resourceType": "Immunization"} if "id" in elements: - resource["id"] = response["id"] - if "meta" in elements and meta: - resource["meta"] = meta + resource_for_bundle["id"] = resource["id"] + if "meta" in elements: + resource_for_bundle["meta"] = meta else: - resource = response["resource"] - resource["meta"] = meta - - entry = BundleEntry( - fullUrl=f"{baseurl}/{response['id']}", - resource=(Immunization.construct(**resource) if _elements else Immunization.parse_obj(resource)), - search=BundleEntrySearch.construct(mode="match") if not _elements else None, + resource_for_bundle = copy.deepcopy(resource) + resource_for_bundle["meta"] = meta + + entry = BundleEntry.construct( + fullUrl=f"{base_url}/{resource['id']}", + resource=( + Immunization.construct(**resource_for_bundle) if elements else Immunization.parse_obj(resource_for_bundle) + ), + search=BundleEntrySearch.construct(mode="match") if not elements else None, ) - fhir_bundle = FhirBundle( - resourceType="Bundle", + return FhirBundle( type="searchset", - link=[BundleLink(relation="self", url=self_url)], + link=[BundleLink(relation="self", url=searched_url)], entry=[entry], total=1, ) - # Reassigned total to ensure it appears last in the response to match expected output - data = json.loads(fhir_bundle.json(by_alias=True)) - data["total"] = data.pop("total") - return data - def check_keys_in_sources(event, not_required_keys): # Decode and parse the body, assuming it is JSON and base64-encoded diff --git a/lambdas/shared/src/common/models/utils/validation_utils.py b/lambdas/shared/src/common/models/utils/validation_utils.py index 3ae5fd32fe..294aa3bdb1 100644 --- a/lambdas/shared/src/common/models/utils/validation_utils.py +++ b/lambdas/shared/src/common/models/utils/validation_utils.py @@ -117,3 +117,7 @@ def validate_resource_versions_match( ) return None + + +def validate_has_status(immunization: dict, status: str) -> bool: + return immunization.get("status") == status diff --git a/lambdas/shared/tests/test_common/models/utils/test_generic_utils.py b/lambdas/shared/tests/test_common/models/utils/test_generic_utils.py index fcd61c8d9f..1f20db3eb2 100644 --- a/lambdas/shared/tests/test_common/models/utils/test_generic_utils.py +++ b/lambdas/shared/tests/test_common/models/utils/test_generic_utils.py @@ -3,84 +3,7 @@ import unittest from datetime import date, datetime -from common.models.utils.generic_utils import form_json -from test_common.testing_utils.generic_utils import format_date_types, load_json_data - - -class TestFormJson(unittest.TestCase): - def setUp(self): - self.baseurl = "https://api.service.nhs.uk/immunisation-fhir-api/Immunization" - self.identifier = "https://supplierABC/identifiers/vacc|f10b59b3-fc73-4616-99c9-9e882ab31184" - self.response = { - "resource": load_json_data("completed_covid_immunization_event.json"), - "id": "f10b59b3-fc73-4616-99c9-9e882ab31184", - "version": "2", - } - - self.maxDiff = None - - def test_no_response(self): - out = form_json(None, None, self.identifier, self.baseurl) - self.assertEqual(out["resourceType"], "Bundle") - self.assertEqual(out["type"], "searchset") - self.assertEqual(out["link"][0]["url"], f"{self.baseurl}?identifier={self.identifier}") - self.assertEqual(out["entry"], []) - self.assertEqual(out["total"], 0) - - def test_identifier_only_returns_full_resource(self): - out = form_json(self.response, None, self.identifier, self.baseurl) - self.assertEqual(out["total"], 1) - self.assertEqual(out["link"][0]["url"], f"{self.baseurl}?identifier={self.identifier}") - self.assertDictEqual(out["entry"][0]["resource"], self.response["resource"]) - self.assertEqual(out["entry"][0]["fullUrl"], f"{self.baseurl}/{self.response['id']}") - - def test_identifier_with_id_element_truncates_to_id(self): - out = form_json(self.response, "id", self.identifier, self.baseurl) - res = out["entry"][0]["resource"] - self.assertEqual(out["total"], 1) - self.assertEqual( - out["link"][0]["url"], - f"{self.baseurl}?identifier={self.identifier}&_elements=id", - ) - self.assertEqual(res["resourceType"], "Immunization") - self.assertEqual(res["id"], self.response["id"]) - self.assertNotIn("meta", res) - - def test_identifier_with_meta_element_truncates_to_meta(self): - out = form_json(self.response, "meta", self.identifier, self.baseurl) - res = out["entry"][0]["resource"] - self.assertEqual(out["total"], 1) - self.assertEqual( - out["link"][0]["url"], - f"{self.baseurl}?identifier={self.identifier}&_elements=meta", - ) - self.assertEqual(res["resourceType"], "Immunization") - self.assertIn("meta", res) - self.assertEqual(res["meta"]["versionId"], self.response["version"]) - - def test_identifier_with_id_and_meta_elements_truncates_both(self): - out = form_json(self.response, "id,meta", self.identifier, self.baseurl) - res = out["entry"][0]["resource"] - self.assertEqual(out["total"], 1) - self.assertEqual( - out["link"][0]["url"], - f"{self.baseurl}?identifier={self.identifier}&_elements=id,meta", - ) - self.assertEqual(res["resourceType"], "Immunization") - self.assertEqual(res["id"], self.response["id"]) - self.assertIn("meta", res) - self.assertEqual(res["meta"]["versionId"], self.response["version"]) - - def test_elements_whitespace_and_case_are_handled(self): - raw_elements = " ID , MeTa " - out = form_json(self.response, raw_elements, self.identifier, self.baseurl) - res = out["entry"][0]["resource"] - self.assertEqual( - out["link"][0]["url"], - f"{self.baseurl}?identifier={self.identifier}&_elements={raw_elements}", - ) - self.assertEqual(res["id"], self.response["id"]) - self.assertEqual(res["meta"]["versionId"], self.response["version"]) +from test_common.testing_utils.generic_utils import format_date_types class TestFormatFutureDates(unittest.TestCase): diff --git a/lambdas/shared/tests/test_common/testing_utils/immunization_utils.py b/lambdas/shared/tests/test_common/testing_utils/immunization_utils.py index d220d46b7b..6e4566a86c 100644 --- a/lambdas/shared/tests/test_common/testing_utils/immunization_utils.py +++ b/lambdas/shared/tests/test_common/testing_utils/immunization_utils.py @@ -14,9 +14,10 @@ def create_covid_immunization(imms_id, nhs_number=VALID_NHS_NUMBER) -> Immunizat def create_covid_immunization_dict( - imms_id, - nhs_number=VALID_NHS_NUMBER, - occurrence_date_time="2021-02-07T13:28:17+00:00", + imms_id: str, + nhs_number: str = VALID_NHS_NUMBER, + occurrence_date_time: str = "2021-02-07T13:28:17+00:00", + status: str = "completed", ): immunization_json = load_json_data("completed_covid_immunization_event.json") immunization_json["id"] = imms_id @@ -26,6 +27,7 @@ def create_covid_immunization_dict( ) immunization_json["occurrenceDateTime"] = occurrence_date_time + immunization_json["status"] = status return immunization_json diff --git a/lambdas/shared/tests/test_common/testing_utils/values_for_tests.py b/lambdas/shared/tests/test_common/testing_utils/values_for_tests.py index a1a3990b00..e577d04f9f 100644 --- a/lambdas/shared/tests/test_common/testing_utils/values_for_tests.py +++ b/lambdas/shared/tests/test_common/testing_utils/values_for_tests.py @@ -242,6 +242,98 @@ class ValidValues: {"use": "old", "family": "Tray", "given": ["Florence"]}, ] + # Search output removes contained array and produces a reference to the patient + expected_resource_in_search = { + "resourceType": "Immunization", + "id": "1234-some-id", + "extension": [ + { + "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-VaccinationProcedure", + "valueCodeableConcept": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "1324681000000101", + "display": "Administration of first dose of severe acute respiratory syndrome coronavirus 2 vaccine (procedure)", + } + ] + }, + } + ], + "identifier": [ + {"use": "official", "system": "https://supplierABC/identifiers/vacc", "value": "ACME-vacc123456"}, + ], + "status": "completed", + "vaccineCode": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "39114911000001105", + "display": "COVID-19 Vaccine Vaxzevria (ChAdOx1 S [recombinant]) not less than 2.5x100,000,000 infectious units/0.5ml dose suspension for injection multidose vials (AstraZeneca UK Ltd) (product)", + } + ] + }, + "patient": { + "reference": "urn:uuid:123456789-12", + "type": "Patient", + "identifier": {"system": "https://fhir.nhs.uk/Id/nhs-number", "value": "9990548609"}, + }, + "occurrenceDateTime": "2021-02-07T13:28:17+00:00", + "recorded": "2021-02-07T13:28:17+00:00", + "primarySource": True, + "location": { + "type": "Location", + "identifier": {"system": "https://fhir.nhs.uk/Id/ods-organization-code", "value": "X99999"}, + }, + "manufacturer": {"display": "AstraZeneca Ltd"}, + "lotNumber": "4120Z001", + "expirationDate": "2021-07-02", + "site": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "368208006", + "display": "Left upper arm structure (body structure)", + } + ] + }, + "route": { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "78421000", + "display": "Intramuscular route (qualifier value)", + } + ] + }, + "doseQuantity": {"value": 0.5, "unit": "milliliter", "system": "http://unitsofmeasure.org", "code": "ml"}, + "performer": [ + { + "actor": { + "type": "Organization", + "identifier": {"system": "https://fhir.nhs.uk/Id/ods-organization-code", "value": "B0C4P"}, + } + } + ], + "reasonCode": [{"coding": [{"system": "http://snomed.info/sct", "code": "443684005"}]}], + "protocolApplied": [ + { + "targetDisease": [ + { + "coding": [ + { + "system": "http://snomed.info/sct", + "code": "840539006", + "display": "Disease caused by severe acute respiratory syndrome coronavirus 2", + } + ] + } + ], + "doseNumberPositiveInt": 1, + } + ], + } + class NameInstances: """Class containing example name instances.""" diff --git a/tests/e2e/test_search_by_identifier_immunization.py b/tests/e2e/test_search_by_identifier_immunization.py index 70593aa87b..a02492c4df 100644 --- a/tests/e2e/test_search_by_identifier_immunization.py +++ b/tests/e2e/test_search_by_identifier_immunization.py @@ -182,8 +182,8 @@ class SearchTestParams(NamedTuple): "POST", f"identifier={identifier_system}|{identifier_value}", f"identifier={identifier_system}|{identifier_value}", - False, - 400, + True, + 200, ), ] for search in searches: diff --git a/tests/e2e/test_search_identifier_elements_immunization.py b/tests/e2e/test_search_identifier_elements_immunization.py index 2f691c67cd..5431c6931d 100644 --- a/tests/e2e/test_search_identifier_elements_immunization.py +++ b/tests/e2e/test_search_identifier_elements_immunization.py @@ -109,8 +109,8 @@ class SearchTestParams(NamedTuple): "POST", f"identifier={identifier_system}|{identifier_value}", f"identifier={identifier_system}|{identifier_value}", - False, - 400, + True, + 200, ), ] for search in searches: diff --git a/tests/e2e/test_search_immunization.py b/tests/e2e/test_search_immunization.py index 990c3d208f..8464653d23 100644 --- a/tests/e2e/test_search_immunization.py +++ b/tests/e2e/test_search_immunization.py @@ -379,7 +379,7 @@ def test_search_immunization_accepts_include_and_provides_patient(self): imms_obj_id = self.store_records(imms_obj) response = self.default_imms_api.search_immunizations_full( - "POST", + "GET", f"patient.identifier={valid_patient_identifier1}&-immunization.target={VaccineTypes.mmr}" + "&_include=Immunization:patient", body=None, @@ -399,7 +399,7 @@ def test_search_immunization_accepts_include_and_provides_patient(self): assert patient_entry["resource"]["identifier"][0]["value"] == valid_nhs_number1 response_without_include = self.default_imms_api.search_immunizations_full( - "POST", + "GET", f"patient.identifier={valid_patient_identifier1}&-immunization.target={VaccineTypes.mmr}", body=None, )