diff --git a/src/eligibility_signposting_api/api_error_response.py b/src/eligibility_signposting_api/api_error_response.py new file mode 100644 index 000000000..9b81a740c --- /dev/null +++ b/src/eligibility_signposting_api/api_error_response.py @@ -0,0 +1,147 @@ +import json +import logging +import uuid +from datetime import UTC, datetime +from enum import Enum +from http import HTTPStatus +from typing import Any + +from fhir.resources.operationoutcome import OperationOutcome, OperationOutcomeIssue + +logger = logging.getLogger(__name__) + + +class FHIRIssueSeverity(str, Enum): + FATAL = "fatal" + ERROR = "error" + WARNING = "warning" + INFORMATION = "information" + + +class FHIRIssueCode(str, Enum): + FORBIDDEN = "forbidden" + PROCESSING = "processing" + VALUE = "value" + + +class FHIRSpineErrorCode(str, Enum): + INVALID_NHS_NUMBER = "INVALID_NHS_NUMBER" + INVALID_PARAMETER = "INVALID_PARAMETER" + INTERNAL_SERVER_ERROR = "INTERNAL_SERVER_ERROR" + REFERENCE_NOT_FOUND = "REFERENCE_NOT_FOUND" + + +class APIErrorResponse: + def __init__( # noqa: PLR0913 + self, + status_code: HTTPStatus, + fhir_issue_code: FHIRIssueCode, + fhir_issue_severity: FHIRIssueSeverity, + fhir_coding_system: str, + fhir_error_code: str, + fhir_display_message: str, + ) -> None: + self.status_code = status_code + self.fhir_issue_code = fhir_issue_code + self.fhir_issue_severity = fhir_issue_severity + self.fhir_coding_system = fhir_coding_system + self.fhir_error_code = fhir_error_code + self.fhir_display_message = fhir_display_message + + def build_operation_outcome_issue(self, diagnostics: str, location: list[str] | None) -> OperationOutcomeIssue: + details = { + "coding": [ + { + "system": self.fhir_coding_system, + "code": self.fhir_error_code, + "display": self.fhir_display_message, + } + ] + } + return OperationOutcomeIssue( + severity=self.fhir_issue_severity, + code=self.fhir_issue_code, + diagnostics=diagnostics, + location=location, + details=details, + ) # pyright: ignore[reportCallIssue] + + def generate_response(self, diagnostics: str, location_param: str | None = None) -> dict[str, Any]: + issue_location = [f"parameters/{location_param}"] if location_param else None + + problem = OperationOutcome( + id=str(uuid.uuid4()), + meta={"lastUpdated": datetime.now(UTC)}, + issue=[self.build_operation_outcome_issue(diagnostics, issue_location)], + ) # pyright: ignore[reportCallIssue] + + response_body = json.dumps(problem.model_dump(by_alias=True, mode="json")) + + return { + "statusCode": self.status_code, + "headers": {"Content-Type": "application/fhir+json"}, + "body": response_body, + } + + def log_and_generate_response( + self, log_message: str, diagnostics: str, location_param: str | None = None + ) -> dict[str, Any]: + logger.error(log_message) + return self.generate_response(diagnostics, location_param) + + +INVALID_INCLUDE_ACTIONS_ERROR = APIErrorResponse( + status_code=HTTPStatus.UNPROCESSABLE_ENTITY, + fhir_issue_code=FHIRIssueCode.VALUE, + fhir_issue_severity=FHIRIssueSeverity.ERROR, + fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER, + fhir_display_message="The supplied value was not recognised by the API.", +) + +INVALID_CATEGORY_ERROR = APIErrorResponse( + status_code=HTTPStatus.UNPROCESSABLE_ENTITY, + fhir_issue_code=FHIRIssueCode.VALUE, + fhir_issue_severity=FHIRIssueSeverity.ERROR, + fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER, + fhir_display_message="The supplied category was not recognised by the API.", +) + +INVALID_CONDITION_FORMAT_ERROR = APIErrorResponse( + status_code=HTTPStatus.BAD_REQUEST, + fhir_issue_code=FHIRIssueCode.VALUE, + fhir_issue_severity=FHIRIssueSeverity.ERROR, + fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER, + fhir_display_message="The given conditions were not in the expected format.", +) + +NHS_NUMBER_NOT_FOUND_ERROR = APIErrorResponse( + status_code=HTTPStatus.NOT_FOUND, + fhir_issue_code=FHIRIssueCode.PROCESSING, + fhir_issue_severity=FHIRIssueSeverity.ERROR, + fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + fhir_error_code=FHIRSpineErrorCode.REFERENCE_NOT_FOUND, + fhir_display_message="The given NHS number was not found in our datasets. " + "This could be because the number is incorrect or " + "some other reason we cannot process that number.", +) + +INTERNAL_SERVER_ERROR = APIErrorResponse( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + fhir_issue_code=FHIRIssueCode.PROCESSING, + fhir_issue_severity=FHIRIssueSeverity.ERROR, + fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + fhir_error_code=FHIRSpineErrorCode.INTERNAL_SERVER_ERROR, + fhir_display_message="An unexpected internal server error occurred.", +) + +NHS_NUMBER_MISMATCH_ERROR = APIErrorResponse( + status_code=HTTPStatus.FORBIDDEN, + fhir_issue_code=FHIRIssueCode.FORBIDDEN, + fhir_issue_severity=FHIRIssueSeverity.ERROR, + fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + fhir_error_code=FHIRSpineErrorCode.INVALID_NHS_NUMBER, + fhir_display_message="The provided NHS number does not match the record.", +) diff --git a/src/eligibility_signposting_api/app.py b/src/eligibility_signposting_api/app.py index 69b01e77e..9073bb779 100644 --- a/src/eligibility_signposting_api/app.py +++ b/src/eligibility_signposting_api/app.py @@ -11,7 +11,7 @@ from eligibility_signposting_api.config.config import config, init_logging from eligibility_signposting_api.error_handler import handle_exception from eligibility_signposting_api.views import eligibility_blueprint -from eligibility_signposting_api.wrapper import validate_matching_nhs_number +from eligibility_signposting_api.wrapper import validate_request_params init_logging() logger = logging.getLogger(__name__) @@ -23,7 +23,7 @@ def main() -> None: # pragma: no cover app.run(debug=config()["log_level"] == logging.DEBUG) -@validate_matching_nhs_number() +@validate_request_params() def lambda_handler(event: LambdaEvent, context: LambdaContext) -> dict[str, Any]: # pragma: no cover """Run the Flask app as an AWS Lambda.""" app = create_app() diff --git a/src/eligibility_signposting_api/error_handler.py b/src/eligibility_signposting_api/error_handler.py index 640bbea42..5ff156d5b 100644 --- a/src/eligibility_signposting_api/error_handler.py +++ b/src/eligibility_signposting_api/error_handler.py @@ -1,12 +1,12 @@ import logging import traceback -from http import HTTPStatus -from fhir.resources.operationoutcome import OperationOutcome, OperationOutcomeIssue from flask import make_response from flask.typing import ResponseReturnValue from werkzeug.exceptions import HTTPException +from eligibility_signposting_api.api_error_response import INTERNAL_SERVER_ERROR + logger = logging.getLogger(__name__) @@ -17,11 +17,8 @@ def handle_exception(e: Exception) -> ResponseReturnValue | HTTPException: if isinstance(e, HTTPException): return e - problem = OperationOutcome( - issue=[ - OperationOutcomeIssue( - severity="severe", code="unexpected", diagnostics="".join(traceback.format_exception(e)) - ) # pyright: ignore[reportCallIssue] - ] + full_traceback = "".join(traceback.format_exception(e)) + response = INTERNAL_SERVER_ERROR.log_and_generate_response( + log_message=f"An unexpected error occurred: {full_traceback}", diagnostics="An unexpected error occurred." ) - return make_response(problem.model_dump(by_alias=True), HTTPStatus.INTERNAL_SERVER_ERROR) + return make_response(response.get("body"), response.get("statusCode")) diff --git a/src/eligibility_signposting_api/views/eligibility.py b/src/eligibility_signposting_api/views/eligibility.py index 628962369..8c91d7de7 100644 --- a/src/eligibility_signposting_api/views/eligibility.py +++ b/src/eligibility_signposting_api/views/eligibility.py @@ -2,18 +2,16 @@ import uuid from datetime import UTC, datetime from http import HTTPStatus -from typing import Never -from fhir.resources.R4B.operationoutcome import OperationOutcome, OperationOutcomeIssue from flask import Blueprint, make_response, request from flask.typing import ResponseReturnValue from wireup import Injected +from eligibility_signposting_api.api_error_response import NHS_NUMBER_NOT_FOUND_ERROR from eligibility_signposting_api.audit.audit_context import AuditContext from eligibility_signposting_api.audit.audit_service import AuditService from eligibility_signposting_api.model.eligibility import Condition, EligibilityStatus, NHSNumber, Status from eligibility_signposting_api.services import EligibilityService, UnknownPersonError -from eligibility_signposting_api.services.eligibility_services import InvalidQueryParamError from eligibility_signposting_api.views.response_model import eligibility from eligibility_signposting_api.views.response_model.eligibility import ProcessedSuggestion @@ -50,8 +48,6 @@ def check_eligibility( eligibility_status = eligibility_service.get_eligibility_status( nhs_number, include_actions_flag=get_include_actions_flag() ) - except InvalidQueryParamError: - return handle_invalid_query_param_error() except UnknownPersonError: return handle_unknown_person_error(nhs_number) else: @@ -62,49 +58,21 @@ def check_eligibility( ) -def handle_unknown_person_error(nhs_number: NHSNumber) -> ResponseReturnValue: - logger.debug("nhs_number %r not found", nhs_number, extra={"nhs_number": nhs_number}) - problem = OperationOutcome( - issue=[ - OperationOutcomeIssue( - severity="information", - code="nhs-number-not-found", - diagnostics=f'NHS Number "{nhs_number}" not found.', - ) # pyright: ignore[reportCallIssue] - ] - ) - return make_response(problem.model_dump(by_alias=True, mode="json"), HTTPStatus.NOT_FOUND) +def get_include_actions_flag() -> bool: + if not request.args.get("includeActions"): + logger.info("Defaulting includeActions query param to Y as no value was provided") + include_actions_flag = True + else: + include_actions_flag = request.args.get("includeActions") == "Y" + return include_actions_flag -def handle_invalid_query_param_error() -> ResponseReturnValue: - logger.debug( - "Invalid query param", - ) - problem = OperationOutcome( - issue=[ - OperationOutcomeIssue( - severity="error", - code="invalid", - diagnostics="Invalid query param key or value.", - ) # pyright: ignore[reportCallIssue] - ] +def handle_unknown_person_error(nhs_number: NHSNumber) -> ResponseReturnValue: + diagnostics = f"NHS Number '{nhs_number}' was not recognised by the Eligibility Signposting API" + response = NHS_NUMBER_NOT_FOUND_ERROR.log_and_generate_response( + log_message=diagnostics, diagnostics=diagnostics, location_param="id" ) - return make_response(problem.model_dump(by_alias=True, mode="json"), HTTPStatus.BAD_REQUEST) - - -def get_include_actions_flag() -> bool: - include_actions = request.args.get("includeActions") - if "includeActions" in request.args: - normalized = include_actions.upper() if include_actions is not None else None - if normalized not in ("Y", "N", None): - raise_invalid_query_param_error() - elif len(request.args) != 0 and "includeActions" not in request.args: - raise_invalid_query_param_error() - return include_actions is None or include_actions.upper() == "Y" - - -def raise_invalid_query_param_error() -> Never: - raise InvalidQueryParamError + return make_response(response.get("body"), response.get("statusCode")) def build_eligibility_response(eligibility_status: EligibilityStatus) -> eligibility.EligibilityResponse: diff --git a/src/eligibility_signposting_api/wrapper.py b/src/eligibility_signposting_api/wrapper.py index f5b0c8b57..c3437afea 100644 --- a/src/eligibility_signposting_api/wrapper.py +++ b/src/eligibility_signposting_api/wrapper.py @@ -1,35 +1,106 @@ import logging +import re from collections.abc import Callable from functools import wraps +from typing import Any from mangum.types import LambdaContext, LambdaEvent +from eligibility_signposting_api.api_error_response import ( + INVALID_CATEGORY_ERROR, + INVALID_CONDITION_FORMAT_ERROR, + INVALID_INCLUDE_ACTIONS_ERROR, + NHS_NUMBER_MISMATCH_ERROR, +) from eligibility_signposting_api.config.contants import NHS_NUMBER_HEADER logger = logging.getLogger(__name__) +condition_pattern = re.compile(r"^\s*[a-zA-Z0-9]+\s*$", re.IGNORECASE) +category_pattern = re.compile(r"^\s*(VACCINATIONS|SCREENING|ALL)\s*$", re.IGNORECASE) +include_actions_pattern = re.compile(r"^\s*([YN])\s*$", re.IGNORECASE) -class MismatchedNHSNumberError(ValueError): - pass +def validate_query_params(query_params: dict[str, str]) -> tuple[bool, dict[str, Any] | None]: + conditions = query_params.get("conditions", "ALL").split(",") + for condition in conditions: + search = re.search(condition_pattern, condition) + if not search: + return False, get_condition_error_response(condition) -def validate_matching_nhs_number() -> Callable: - def decorator(func: Callable) -> Callable: # pragma: no cover + category = query_params.get("category", "ALL") + if not re.search(category_pattern, category): + return False, get_category_error_response(category) + + include_actions = query_params.get("includeActions", "Y") + if not re.search(include_actions_pattern, include_actions): + return False, get_include_actions_error_response(include_actions) + + return True, None + + +def validate_nhs_number(path_nhs: str, header_nhs: str) -> bool: + logger.info("NHS numbers from the request", extra={"header_nhs": header_nhs, "path_nhs": path_nhs}) + + if not header_nhs or not path_nhs: + logger.error("NHS number is not present", extra={"header_nhs": header_nhs, "path_nhs": path_nhs}) + return False + + if header_nhs != path_nhs: + logger.error("NHS number mismatch", extra={"header_nhs": header_nhs, "path_nhs": path_nhs}) + return False + return True + + +def validate_request_params() -> Callable: + def decorator(func: Callable) -> Callable: @wraps(func) - def wrapper(event: LambdaEvent, context: LambdaContext) -> dict[str, int | str]: - headers = event.get("headers", {}) - path_params = event.get("pathParameters", {}) + def wrapper(event: LambdaEvent, context: LambdaContext) -> dict[str, Any] | None: + path_nhs_no = event.get("pathParameters", {}).get("id") + header_nhs_no = event.get("headers", {}).get(NHS_NUMBER_HEADER) - header_nhs = headers.get(NHS_NUMBER_HEADER) - path_nhs = path_params.get("id") + if not validate_nhs_number(path_nhs_no, header_nhs_no): + message = f"NHS Number {path_nhs_no or ''} does not match the header NHS Number {header_nhs_no or ''}" + return NHS_NUMBER_MISMATCH_ERROR.log_and_generate_response( + log_message=message, diagnostics=message, location_param="id" + ) - logger.info("nhs numbers from the request", extra={"header_nhs": header_nhs, "path_nhs": path_nhs}) + query_params = event.get("queryStringParameters") + if query_params: + is_valid, problem = validate_query_params(query_params) + if not is_valid: + return problem - if header_nhs != path_nhs: - logger.error("NHS number mismatch", extra={"header_nhs_no": header_nhs, "path_nhs_no": path_nhs}) - return {"statusCode": 403, "body": "NHS number mismatch"} return func(event, context) return wrapper return decorator + + +def get_include_actions_error_response(include_actions: str) -> dict[str, Any]: + diagnostics = f"{include_actions} is not a value that is supported by the API" + return INVALID_INCLUDE_ACTIONS_ERROR.log_and_generate_response( + log_message=f"Invalid include actions query param: '{include_actions}'", + diagnostics=diagnostics, + location_param="includeActions", + ) + + +def get_category_error_response(category: str) -> dict[str, Any]: + diagnostics = f"{category} is not a category that is supported by the API" + return INVALID_CATEGORY_ERROR.log_and_generate_response( + log_message=f"Invalid category query param: '{category}'", diagnostics=diagnostics, location_param="category" + ) + + +def get_condition_error_response(condition: str) -> dict[str, Any]: + diagnostics = ( + f"{condition} should be a single or comma separated list of condition " + f"strings with no other punctuation or special characters" + ) + return INVALID_CONDITION_FORMAT_ERROR.log_and_generate_response( + log_message=f"Invalid condition query param: '{condition}'", + diagnostics=diagnostics, + location_param="conditions", + ) diff --git a/tests/integration/in_process/test_eligibility_endpoint.py b/tests/integration/in_process/test_eligibility_endpoint.py index 0a97e474d..5356eed59 100644 --- a/tests/integration/in_process/test_eligibility_endpoint.py +++ b/tests/integration/in_process/test_eligibility_endpoint.py @@ -60,7 +60,7 @@ def test_not_base_eligible( # Given # When - response = client.get(f"/patient-check/{persisted_person_no_cohorts}") + response = client.get(f"/patient-check/{persisted_person_no_cohorts}?includeActions=Y") # Then assert_that( @@ -103,7 +103,7 @@ def test_not_eligible_by_rule( # Given # When - response = client.get(f"/patient-check/{persisted_person_pc_sw19}") + response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y") # Then assert_that( @@ -146,7 +146,7 @@ def test_not_actionable( # Given # When - response = client.get(f"/patient-check/{persisted_person}") + response = client.get(f"/patient-check/{persisted_person}?includeActions=Y") # Then assert_that( @@ -195,7 +195,7 @@ def test_actionable( # Given # When - response = client.get(f"/patient-check/{persisted_77yo_person}") + response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y") # Then assert_that( @@ -248,7 +248,7 @@ def test_not_eligible_by_rule_when_only_magic_cohort_is_present( # Given # When - response = client.get(f"/patient-check/{persisted_person_pc_sw19}") + response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y") # Then assert_that( @@ -291,7 +291,7 @@ def test_not_actionable_when_only_magic_cohort_is_present( # Given # When - response = client.get(f"/patient-check/{persisted_person}") + response = client.get(f"/patient-check/{persisted_person}?includeActions=Y") # Then assert_that( @@ -340,7 +340,7 @@ def test_actionable_when_only_magic_cohort_is_present( # Given # When - response = client.get(f"/patient-check/{persisted_77yo_person}") + response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y") # Then assert_that( @@ -393,7 +393,7 @@ def test_not_base_eligible( # Given # When - response = client.get(f"/patient-check/{persisted_person_no_cohorts}") + response = client.get(f"/patient-check/{persisted_person_no_cohorts}?includeActions=Y") # Then assert_that( @@ -430,7 +430,7 @@ def test_not_eligible_by_rule( # Given # When - response = client.get(f"/patient-check/{persisted_person_pc_sw19}") + response = client.get(f"/patient-check/{persisted_person_pc_sw19}?includeActions=Y") # Then assert_that( @@ -467,7 +467,7 @@ def test_not_actionable( # Given # When - response = client.get(f"/patient-check/{persisted_person}") + response = client.get(f"/patient-check/{persisted_person}?includeActions=Y") # Then assert_that( @@ -510,7 +510,7 @@ def test_actionable( # Given # When - response = client.get(f"/patient-check/{persisted_77yo_person}") + response = client.get(f"/patient-check/{persisted_77yo_person}?includeActions=Y") # Then assert_that( diff --git a/tests/integration/lambda/test_app_running_as_lambda.py b/tests/integration/lambda/test_app_running_as_lambda.py index be536877e..2e712a076 100644 --- a/tests/integration/lambda/test_app_running_as_lambda.py +++ b/tests/integration/lambda/test_app_running_as_lambda.py @@ -135,9 +135,21 @@ def test_install_and_call_flask_lambda_with_unknown_nhs_number( resourceType="OperationOutcome", issue=contains_exactly( has_entries( - severity="information", - code="nhs-number-not-found", - diagnostics=f'NHS Number "{nhs_number}" not found.', + severity="error", + code="processing", + diagnostics=f"NHS Number '{nhs_number!s}' was not " + f"recognised by the Eligibility Signposting API", + details={ + "coding": [ + { + "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + "code": "REFERENCE_NOT_FOUND", + "display": "The given NHS number was not found in our datasets. " + "This could be because the number is incorrect or " + "some other reason we cannot process that number.", + } + ] + }, ) ), ) @@ -146,7 +158,10 @@ def test_install_and_call_flask_lambda_with_unknown_nhs_number( ) messages = get_log_messages(flask_function, logs_client) - assert_that(messages, has_item(contains_string(f"nhs_number '{nhs_number}' not found"))) + assert_that( + messages, + has_item(contains_string(f"NHS Number '{nhs_number}' was not recognised by the Eligibility Signposting API")), + ) def get_log_messages(flask_function: str, logs_client: BaseClient) -> list[str]: @@ -170,6 +185,8 @@ def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers_and_check_i s3_client: BaseClient, audit_bucket: BucketName, api_gateway_endpoint: URL, + flask_function: str, + logs_client: BaseClient, ): # Given # When @@ -244,6 +261,12 @@ def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers_and_check_i assert_that(audit_data["response"]["lastUpdated"], is_not(equal_to(""))) assert_that(audit_data["response"]["condition"], equal_to(expected_conditions)) + messages = get_log_messages(flask_function, logs_client) + assert_that( + messages, + has_item(contains_string("Defaulting includeActions query param to Y as no value was provided")), + ) + def test_given_nhs_number_in_path_does_not_match_with_nhs_number_in_headers_results_in_error_response( lambda_client: BaseClient, # noqa:ARG001 @@ -263,9 +286,119 @@ def test_given_nhs_number_in_path_does_not_match_with_nhs_number_in_headers_resu # Then assert_that( response, - is_response().with_status_code(HTTPStatus.FORBIDDEN).and_body("NHS number mismatch"), + is_response() + .with_status_code(HTTPStatus.FORBIDDEN) + .and_body( + is_json_that( + has_entries( + resourceType="OperationOutcome", + issue=contains_exactly( + has_entries( + severity="error", + code="forbidden", + diagnostics=f"NHS Number {persisted_person} does " + f"not match the header NHS Number 123{persisted_person!s}", + details={ + "coding": [ + { + "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + "code": "INVALID_NHS_NUMBER", + "display": "The provided NHS number does not match the record.", + } + ] + }, + ) + ), + ) + ) + ), + ) + + +def test_given_nhs_number_not_present_in_headers_results_in_error_response( + lambda_client: BaseClient, # noqa:ARG001 + persisted_person: NHSNumber, + campaign_config: CampaignConfig, # noqa:ARG001 + api_gateway_endpoint: URL, +): + # Given + # When + invoke_url = f"{api_gateway_endpoint}/patient-check/{persisted_person}" + response = httpx.get( + invoke_url, + timeout=10, ) + # Then + assert_that( + response, + is_response() + .with_status_code(HTTPStatus.FORBIDDEN) + .and_body( + is_json_that( + has_entries( + resourceType="OperationOutcome", + issue=contains_exactly( + has_entries( + severity="error", + code="forbidden", + diagnostics=f"NHS Number {persisted_person} does not match the header NHS Number ", + details={ + "coding": [ + { + "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + "code": "INVALID_NHS_NUMBER", + "display": "The provided NHS number does not match the record.", + } + ] + }, + ) + ), + ) + ) + ), + ) + + +def test_validation_of_query_params_when_all_are_valid( + lambda_client: BaseClient, # noqa:ARG001 + persisted_person: NHSNumber, + campaign_config: CampaignConfig, # noqa:ARG001 + api_gateway_endpoint: URL, +): + # Given + # When + invoke_url = f"{api_gateway_endpoint}/patient-check/{persisted_person}" + response = httpx.get( + invoke_url, + headers={"nhs-login-nhs-number": persisted_person}, + params={"category": "VACCINATIONS", "conditions": "COVID19", "includeActions": "N"}, + timeout=10, + ) + + # Then + assert_that(response, is_response().with_status_code(HTTPStatus.OK)) + + +def test_validation_of_query_params_when_invalid_conditions_is_specified( + lambda_client: BaseClient, # noqa:ARG001 + persisted_person: NHSNumber, + campaign_config: CampaignConfig, # noqa:ARG001 + api_gateway_endpoint: URL, +): + # Given + # When + invoke_url = f"{api_gateway_endpoint}/patient-check/{persisted_person}" + response = httpx.get( + invoke_url, + headers={"nhs-login-nhs-number": persisted_person}, + params={"category": "ALL", "conditions": "23-097"}, + timeout=10, + ) + + # Then + assert_that(response, is_response().with_status_code(HTTPStatus.BAD_REQUEST)) + def test_given_person_has_unique_status_for_different_conditions_with_audit( # noqa: PLR0913 lambda_client: BaseClient, # noqa:ARG001 diff --git a/tests/unit/test_wrapper.py b/tests/unit/test_wrapper.py new file mode 100644 index 000000000..18ef5d477 --- /dev/null +++ b/tests/unit/test_wrapper.py @@ -0,0 +1,313 @@ +import json +import logging +from http import HTTPStatus + +import pytest + +from eligibility_signposting_api import wrapper +from eligibility_signposting_api.wrapper import logger + + +@pytest.fixture(autouse=True) +def setup_logging_for_tests(): + logger.handlers = [] + logger.setLevel(logging.INFO) + logger.addHandler(logging.NullHandler()) + + +@pytest.mark.parametrize( + ("path_nhs", "header_nhs", "expected_result", "expected_log_msg"), + [ + (None, None, False, "NHS number is not present"), + ("1234567890", None, False, "NHS number is not present"), + (None, "1234567890", False, "NHS number is not present"), + ("1234567890", "0987654321", False, "NHS number mismatch"), + ("1234567890", "1234567890", True, None), + ], +) +def test_validate_nhs_number(path_nhs, header_nhs, expected_result, expected_log_msg, caplog): + with caplog.at_level(logging.ERROR): + result = wrapper.validate_nhs_number(path_nhs, header_nhs) + + assert result == expected_result + + if expected_log_msg: + assert any(expected_log_msg in record.message for record in caplog.records) + else: + assert not caplog.records + + +@pytest.mark.parametrize( + ("conditions_input", "is_valid_expected", "expected_log_msg"), + [ + ("ALL", True, None), + ("COVID", True, None), + ("covid19", True, None), + ("FLU,MMR", True, None), + (" RSV , COVID19", True, None), + (" condition_with_spaces ", False, "Invalid condition query param: ' condition_with_spaces '"), + ("CONDITION_A,ANOTHER_ONE,123ABC", False, "Invalid condition query param: 'CONDITION_A'"), + ("condition1,", False, "Invalid condition query param: ''"), + (",condition2", False, "Invalid condition query param: ''"), + ("condition-invalid", False, "Invalid condition query param: 'condition-invalid'"), + ("condition with spaces", False, "Invalid condition query param: 'condition with spaces'"), + ("condition!", False, "Invalid condition query param: 'condition!'"), + ("condition@#$", False, "Invalid condition query param: 'condition@#$'"), + ], +) +def test_validate_query_params_conditions(conditions_input, is_valid_expected, expected_log_msg, caplog): + params = {"conditions": conditions_input} + + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + + assert is_valid == is_valid_expected + if is_valid_expected: + assert problem is None + assert not caplog.records + else: + assert problem is not None + assert any((record.levelname == "ERROR" and expected_log_msg in record.message) for record in caplog.records) + + +def test_validate_query_params_conditions_default(caplog): + params = {"category": "ALL", "includeActions": "Y"} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid is True + assert problem is None + assert not caplog.records + + +@pytest.mark.parametrize( + ("category_input", "is_valid_expected", "expected_log_msg"), + [ + ("VACCINATIONS", True, None), + ("SCREENING", True, None), + ("ALL", True, None), + ("vaccinations", True, None), + ("screening", True, None), + ("all", True, None), + (" VACCINATIONS ", True, None), + ("OTHER_CATEGORY ", False, "Invalid category query param: 'OTHER_CATEGORY '"), + ("invalid!", False, "Invalid category query param: 'invalid!'"), + ("VACCINATION", False, "Invalid category query param: 'VACCINATION'"), + ], +) +def test_validate_query_params_category(category_input, is_valid_expected, expected_log_msg, caplog): + params = {"category": category_input} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid == is_valid_expected + + if is_valid_expected: + assert problem is None + assert not caplog.records + else: + assert problem is not None + assert any((record.levelname == "ERROR" and expected_log_msg in record.message) for record in caplog.records) + + +def test_validate_query_params_category_default(caplog): + params = {"conditions": "ALL", "includeActions": "Y"} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid is True + assert problem is None + assert not caplog.records + + +@pytest.mark.parametrize( + ("include_actions_input", "is_valid_expected", "expected_log_msg"), + [ + ("Y", True, None), + ("N", True, None), + ("y", True, None), + ("n", True, None), + ("n ", True, None), + ("TRUE", False, "Invalid include actions query param: 'TRUE'"), + ("YES", False, "Invalid include actions query param: 'YES'"), + ("0", False, "Invalid include actions query param: '0'"), + ("1", False, "Invalid include actions query param: '1'"), + ("", False, "Invalid include actions query param: ''"), + (" ", False, "Invalid include actions query param: ' '"), + ], +) +def test_validate_query_params_include_actions(include_actions_input, is_valid_expected, expected_log_msg, caplog): + params = {"includeActions": include_actions_input} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid == is_valid_expected + + if is_valid_expected: + assert problem is None + assert not caplog.records + else: + assert problem is not None + assert any((record.levelname == "ERROR" and expected_log_msg in record.message) for record in caplog.records) + + +def test_validate_query_params_include_actions_default(caplog): + params = {"conditions": "ALL", "category": "ALL"} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid is True + assert problem is None + assert not caplog.records + + +def test_validate_query_params_all_valid_params(caplog): + params = {"conditions": "COND1,COND2", "category": "SCREENING", "includeActions": "N"} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid is True + assert problem is None + assert not caplog.records + + +def test_validate_query_params_mixed_valid_invalid_conditions_fail_first(caplog): + params = {"conditions": "VALID_COND,INVALID!,ANOTHER_VALID", "category": "SCREENING", "includeActions": "N"} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid is False + assert problem is not None + assert any( + (record.levelname == "ERROR" and "Invalid condition query param: " in record.message) + for record in caplog.records + ) + + +def test_validate_query_params_valid_conditions_invalid_category_fail_second(caplog): + params = {"conditions": "CONDITION", "category": "BAD_CAT", "includeActions": "N"} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid is False + assert problem is not None + assert any( + (record.levelname == "ERROR" and "Invalid category query param: " in record.message) + for record in caplog.records + ) + error_logs = [r for r in caplog.records if r.levelname == "ERROR"] + assert len(error_logs) == 1 + + +def test_validate_query_params_valid_conditions_category_invalid_actions_fail_third(caplog): + params = {"conditions": "CONDITION", "category": "VACCINATIONS", "includeActions": "Nope"} + with caplog.at_level(logging.ERROR): + is_valid, problem = wrapper.validate_query_params(params) + assert is_valid is False + assert problem is not None + assert any( + (record.levelname == "ERROR" and "Invalid include actions query param: " in record.message) + for record in caplog.records + ) + error_logs = [r for r in caplog.records if r.levelname == "ERROR"] + assert len(error_logs) == 1 + + +def test_validate_query_params_returns_correct_problem_details_for_conditions_error(): + invalid_condition = "FLU&COVID" + params = {"conditions": invalid_condition} + + is_valid, problem = wrapper.validate_query_params(params) + + assert is_valid is False + assert problem is not None + assert problem["statusCode"] == HTTPStatus.BAD_REQUEST + assert problem["headers"]["Content-Type"] == "application/fhir+json" + + response_body = json.loads(problem["body"]) + + assert response_body["resourceType"] == "OperationOutcome" + assert "id" in response_body + assert "meta" in response_body + assert "lastUpdated" in response_body["meta"] + + assert len(response_body["issue"]) == 1 + issue = response_body["issue"][0] + + assert issue["severity"] == "error" + assert issue["code"] == "value" + assert issue["diagnostics"] == ( + f"{invalid_condition} should be a single or comma separated list of condition " + f"strings with no other punctuation or special characters" + ) + assert issue["location"] == ["parameters/conditions"] + assert "details" in issue + assert "coding" in issue["details"] + assert len(issue["details"]["coding"]) == 1 + coding = issue["details"]["coding"][0] + + assert coding["system"] == "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1" + assert coding["code"] == "INVALID_PARAMETER" + assert coding["display"] == "The given conditions were not in the expected format." + + +def test_validate_query_params_returns_correct_problem_details_for_category_error(): + invalid_category = "HEALTHCHECKS" + params = {"category": invalid_category} + + is_valid, problem = wrapper.validate_query_params(params) + + assert is_valid is False + assert problem is not None + assert problem["statusCode"] == HTTPStatus.UNPROCESSABLE_ENTITY + assert problem["headers"]["Content-Type"] == "application/fhir+json" + + response_body = json.loads(problem["body"]) + + assert response_body["resourceType"] == "OperationOutcome" + assert "id" in response_body + assert "meta" in response_body + assert "lastUpdated" in response_body["meta"] + + assert len(response_body["issue"]) == 1 + issue = response_body["issue"][0] + + assert issue["severity"] == "error" + assert issue["code"] == "value" + assert issue["diagnostics"] == f"{invalid_category} is not a category that is supported by the API" + assert issue["location"] == ["parameters/category"] + assert "details" in issue + assert "coding" in issue["details"] + assert len(issue["details"]["coding"]) == 1 + coding = issue["details"]["coding"][0] + + assert coding["system"] == "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1" + assert coding["code"] == "INVALID_PARAMETER" + assert coding["display"] == "The supplied category was not recognised by the API." + + +def test_validate_query_params_returns_correct_problem_details_for_include_actions_error(): + invalid_include_actions = "NAH" + params = {"includeActions": invalid_include_actions} + + is_valid, problem = wrapper.validate_query_params(params) + + assert is_valid is False + assert problem is not None + assert problem["statusCode"] == HTTPStatus.UNPROCESSABLE_ENTITY + assert problem["headers"]["Content-Type"] == "application/fhir+json" + + response_body = json.loads(problem["body"]) + + assert response_body["resourceType"] == "OperationOutcome" + assert "id" in response_body + assert "meta" in response_body + assert "lastUpdated" in response_body["meta"] + + assert len(response_body["issue"]) == 1 + issue = response_body["issue"][0] + + assert issue["severity"] == "error" + assert issue["code"] == "value" + assert issue["diagnostics"] == f"{invalid_include_actions} is not a value that is supported by the API" + assert issue["location"] == ["parameters/includeActions"] + assert "details" in issue + assert "coding" in issue["details"] + assert len(issue["details"]["coding"]) == 1 + coding = issue["details"]["coding"][0] + + assert coding["system"] == "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1" + assert coding["code"] == "INVALID_PARAMETER" + assert coding["display"] == "The supplied value was not recognised by the API." diff --git a/tests/unit/views/test_eligibility.py b/tests/unit/views/test_eligibility.py index b8a140d80..99b0fac34 100644 --- a/tests/unit/views/test_eligibility.py +++ b/tests/unit/views/test_eligibility.py @@ -2,13 +2,13 @@ import logging from datetime import UTC, datetime from http import HTTPStatus -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, patch from uuid import uuid4 import pytest from brunns.matchers.data import json_matching as is_json_that from brunns.matchers.werkzeug import is_werkzeug_response as is_response -from flask import Flask, Request +from flask import Flask from flask.testing import FlaskClient from hamcrest import assert_that, contains_exactly, has_entries, has_length, is_, none from wireup.integration.flask import get_app_container @@ -33,12 +33,10 @@ UrlLink, ) from eligibility_signposting_api.services import EligibilityService, UnknownPersonError -from eligibility_signposting_api.services.eligibility_services import InvalidQueryParamError from eligibility_signposting_api.views.eligibility import ( build_actions, build_eligibility_cohorts, build_suitability_results, - get_include_actions_flag, ) from eligibility_signposting_api.views.response_model import eligibility from tests.fixtures.builders.model.eligibility import ( @@ -125,7 +123,20 @@ def test_no_nhs_number_given(app: Flask, client: FlaskClient): resourceType="OperationOutcome", issue=contains_exactly( has_entries( - severity="information", code="nhs-number-not-found", diagnostics='NHS Number "" not found.' + severity="error", + code="processing", + diagnostics="NHS Number '' was not recognised by the Eligibility Signposting API", + details={ + "coding": [ + { + "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + "code": "REFERENCE_NOT_FOUND", + "display": "The given NHS number was not found in our datasets. " + "This could be because the number is incorrect or " + "some other reason we cannot process that number.", + } + ] + }, ) ), ) @@ -138,6 +149,7 @@ def test_unexpected_error(app: Flask, client: FlaskClient): # Given with get_app_container(app).override.service(EligibilityService, new=FakeUnexpectedErrorEligibilityService()): response = client.get("/patient-check/12345") + assert_that( response, is_response() @@ -146,7 +158,22 @@ def test_unexpected_error(app: Flask, client: FlaskClient): is_json_that( has_entries( resourceType="OperationOutcome", - issue=contains_exactly(has_entries(severity="severe", code="unexpected")), + issue=contains_exactly( + has_entries( + severity="error", + code="processing", + diagnostics="An unexpected error occurred.", + details={ + "coding": [ + { + "system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1", + "code": "INTERNAL_SERVER_ERROR", + "display": "An unexpected internal server error occurred.", + } + ] + }, + ) + ), ) ) ), @@ -449,142 +476,6 @@ def test_build_actions(suggested_actions, expected): assert_that(results, contains_exactly(*expected)) -def test_nhs_number_and_include_actions_param_given_and_is_yes(app: Flask, client: FlaskClient): - # Given - with ( - get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()), - get_app_container(app).override.service(AuditService, new=FakeAuditService()), - ): - # When - response = client.get("/patient-check/12345?includeActions=Y") - - # Then - assert_that(response, is_response().with_status_code(HTTPStatus.OK)) - - -def test_nhs_number_and_include_actions_param_no_given(app: Flask, client: FlaskClient): - # Given - with ( - get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()), - get_app_container(app).override.service(AuditService, new=FakeAuditService()), - ): - # When - response = client.get("/patient-check/12345?includeActions=N") - - # Then - assert_that(response, is_response().with_status_code(HTTPStatus.OK)) - - -def test_nhs_number_and_include_actions_param_value_is_incorrect(app: Flask, client: FlaskClient): - # Given - with get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()): - # When - response = client.get("/patient-check/12345?includeActions=abc") - - # Then - assert_that( - response, - is_response() - .with_status_code(HTTPStatus.BAD_REQUEST) - .and_text( - is_json_that( - has_entries( - resourceType="OperationOutcome", - issue=contains_exactly( - has_entries( - severity="error", code="invalid", diagnostics="Invalid query param key or value." - ) - ), - ) - ) - ), - ) - - -def test_nhs_number_and_unrecognised_query_param_provided(app: Flask, client: FlaskClient): - # Given - with get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()): - # When - response = client.get("/patient-check/12345?example-key=example-value") - - # Then - assert_that( - response, - is_response() - .with_status_code(HTTPStatus.BAD_REQUEST) - .and_text( - is_json_that( - has_entries( - resourceType="OperationOutcome", - issue=contains_exactly( - has_entries( - severity="error", code="invalid", diagnostics="Invalid query param key or value." - ) - ), - ) - ) - ), - ) - - -def test_query_param_include_actions_flag_with_yes(): - # Given: - mock_request = Mock(spec=Request) - mock_request.args = {"includeActions": "Y"} - with patch("eligibility_signposting_api.views.eligibility.request", mock_request): - # When: - result = get_include_actions_flag() - - # Then: - assert result is True - - -def test_query_param_include_actions_flag_with_no(): - # Given: - mock_request = Mock(spec=Request) - mock_request.args = {"includeActions": "N"} - with patch("eligibility_signposting_api.views.eligibility.request", mock_request): - # When: - result = get_include_actions_flag() - - # Then: - assert result is False - - -def test_query_param_include_actions_flag_with_no_param(): - # Given: - mock_request = Mock(spec=Request) - mock_request.args = {} - with patch("eligibility_signposting_api.views.eligibility.request", mock_request): - # When: - result = get_include_actions_flag() - - # Then: - assert result is True - - -def test_query_param_include_actions_flag_with_invalid_value(): - # Given: - mock_request = Mock(spec=Request) - mock_request.args = {"includeActions": "invalid"} - with ( - patch("eligibility_signposting_api.views.eligibility.request", mock_request), - pytest.raises(InvalidQueryParamError), - ): - get_include_actions_flag() - - -def test_query_param_include_actions_flag_with_other_params(): - # Given: - mock_request = Mock(spec=Request) - mock_request.args = {"otherParam": "value"} - with ( - patch("eligibility_signposting_api.views.eligibility.request", mock_request), - pytest.raises(InvalidQueryParamError), - ): - get_include_actions_flag() - - def test_excludes_nulls_via_build_response(client: FlaskClient): mocked_response = eligibility.EligibilityResponse( responseId=uuid4(),