Skip to content

Commit ebdf18e

Browse files
Akol125Akol125
authored
VED-375 Search Parameter Error (#854)
1 parent 6608372 commit ebdf18e

File tree

3 files changed

+194
-75
lines changed

3 files changed

+194
-75
lines changed

‎backend/src/fhir_service.py‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,11 +332,6 @@ def search_immunizations(
332332
Bundles the resources with the relevant patient resource and returns the bundle along with a boolean to state
333333
whether the supplier requested vaccine types they were not authorised for.
334334
"""
335-
# TODO: is disease type a mandatory field? (I assumed it is)
336-
# i.e. Should we provide a search option for getting Patient's entire imms history?
337-
if not nhs_number_mod11_check(nhs_number):
338-
return create_diagnostics(), False
339-
340335
permitted_vacc_types = self.authoriser.filter_permitted_vacc_types(
341336
supplier_system, ApiOperationCode.SEARCH, set(vaccine_types)
342337
)

‎backend/src/parameter_parser.py‎

Lines changed: 105 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from clients import redis_client, logger
1010
from models.errors import ParameterException
11+
from models.utils.generic_utils import nhs_number_mod11_check
1112
from models.constants import Constants
1213

1314
ParamValue = list[str]
@@ -35,6 +36,110 @@ class SearchParams:
3536
def __repr__(self):
3637
return str(self.__dict__)
3738

39+
def process_patient_identifier(identifier_params: ParamContainer) -> str:
40+
"""Validate and parse patient identifier parameter.
41+
42+
:raises ParameterException:
43+
"""
44+
patient_identifiers = identifier_params.get(patient_identifier_key, [])
45+
patient_identifier = patient_identifiers[0] if len(patient_identifiers) == 1 else None
46+
47+
if patient_identifier is None:
48+
raise ParameterException(f"Search parameter {patient_identifier_key} must have one value.")
49+
50+
patient_identifier_parts = patient_identifier.split("|")
51+
identifier_system = patient_identifier_parts[0]
52+
if len(patient_identifier_parts) != 2 or identifier_system != patient_identifier_system:
53+
raise ParameterException("patient.identifier must be in the format of "
54+
f"\"{patient_identifier_system}|{{NHS number}}\" "
55+
f"e.g. \"{patient_identifier_system}|9000000009\"")
56+
57+
nhs_number = patient_identifier_parts[1]
58+
if not nhs_number_mod11_check(nhs_number):
59+
raise ParameterException("Search parameter patient.identifier must be a valid NHS number.")
60+
61+
return nhs_number
62+
63+
64+
def process_immunization_target(imms_params: ParamContainer) -> list[str]:
65+
"""Validate and parse immunization target parameter.
66+
67+
:raises ParameterException:
68+
"""
69+
vaccine_types = [vaccine_type for vaccine_type in set(imms_params.get(immunization_target_key, [])) if
70+
vaccine_type is not None]
71+
if len(vaccine_types) < 1:
72+
raise ParameterException(f"Search parameter {immunization_target_key} must have one or more values.")
73+
74+
valid_vaccine_types = redis_client.hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY)
75+
if any(x not in valid_vaccine_types for x in vaccine_types):
76+
raise ParameterException(
77+
f"immunization-target must be one or more of the following: {', '.join(valid_vaccine_types)}")
78+
79+
return vaccine_types
80+
81+
82+
def process_mandatory_params(params: ParamContainer) -> tuple[str, list[str]]:
83+
"""Validate mandatory params and return (patient_identifier, vaccine_types).
84+
Raises ParameterException for any validation error.
85+
"""
86+
# patient.identifier
87+
patient_identifier = process_patient_identifier(params)
88+
89+
# immunization.target
90+
vaccine_types = process_immunization_target(params)
91+
92+
return patient_identifier, vaccine_types
93+
94+
95+
def process_optional_params(params: ParamContainer) -> tuple[datetime.date, datetime.date, Optional[str], list[str]]:
96+
"""Parse optional params (date.from, date.to, _include).
97+
Returns (date_from, date_to, include, errors).
98+
"""
99+
errors: list[str] = []
100+
date_from = None
101+
date_to = None
102+
103+
date_froms = params.get(date_from_key, [])
104+
if len(date_froms) > 1:
105+
errors.append(f"Search parameter {date_from_key} may have one value at most.")
106+
107+
try:
108+
date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date() if len(date_froms) == 1 else date_from_default
109+
except ValueError:
110+
errors.append(f"Search parameter {date_from_key} must be in format: YYYY-MM-DD")
111+
112+
date_tos = params.get(date_to_key, [])
113+
if len(date_tos) > 1:
114+
errors.append(f"Search parameter {date_to_key} may have one value at most.")
115+
116+
try:
117+
date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date() if len(date_tos) == 1 else date_to_default
118+
except ValueError:
119+
errors.append(f"Search parameter {date_to_key} must be in format: YYYY-MM-DD")
120+
121+
includes = params.get(include_key, [])
122+
if includes and includes[0].lower() != "immunization:patient":
123+
errors.append(f"Search parameter {include_key} may only be 'Immunization:patient' if provided.")
124+
include = includes[0] if len(includes) > 0 else None
125+
126+
return date_from, date_to, include, errors
127+
128+
def process_search_params(params: ParamContainer) -> SearchParams:
129+
"""Validate and parse search parameters.
130+
:raises ParameterException:
131+
"""
132+
patient_identifier, vaccine_types = process_mandatory_params(params)
133+
date_from, date_to, include, errors = process_optional_params(params)
134+
135+
if date_from and date_to and date_from > date_to:
136+
errors.append(f"Search parameter {date_from_key} must be before {date_to_key}")
137+
138+
if errors:
139+
raise ParameterException("; ".join(errors))
140+
141+
return SearchParams(patient_identifier, vaccine_types, date_from, date_to, include)
142+
38143

39144
def process_params(aws_event: APIGatewayProxyEventV1) -> ParamContainer:
40145
"""Combines query string and content parameters. Duplicates not allowed. Splits on a comma."""
@@ -80,74 +185,6 @@ def parse_body_params(aws_event: APIGatewayProxyEventV1) -> ParamContainer:
80185
return parsed_params
81186

82187

83-
def process_search_params(params: ParamContainer) -> SearchParams:
84-
"""Validate and parse search parameters.
85-
86-
:raises ParameterException:
87-
"""
88-
89-
# patient.identifier
90-
patient_identifiers = params.get(patient_identifier_key, [])
91-
patient_identifier = patient_identifiers[0] if len(patient_identifiers) == 1 else None
92-
93-
if patient_identifier is None:
94-
raise ParameterException(f"Search parameter {patient_identifier_key} must have one value.")
95-
96-
patient_identifier_parts = patient_identifier.split("|")
97-
if len(patient_identifier_parts) != 2 or not patient_identifier_parts[
98-
0] == patient_identifier_system:
99-
raise ParameterException("patient.identifier must be in the format of "
100-
f"\"{patient_identifier_system}|{{NHS number}}\" "
101-
f"e.g. \"{patient_identifier_system}|9000000009\"")
102-
103-
patient_identifier = patient_identifier.split("|")[1]
104-
105-
# immunization.target
106-
params[immunization_target_key] = list(set(params.get(immunization_target_key, [])))
107-
vaccine_types = [vaccine_type for vaccine_type in params[immunization_target_key] if
108-
vaccine_type is not None]
109-
if len(vaccine_types) < 1:
110-
raise ParameterException(f"Search parameter {immunization_target_key} must have one or more values.")
111-
112-
valid_vaccine_types = redis_client.hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY)
113-
if any(x not in valid_vaccine_types for x in vaccine_types):
114-
raise ParameterException(
115-
f"immunization-target must be one or more of the following: {', '.join(valid_vaccine_types)}")
116-
117-
# date.from
118-
date_froms = params.get(date_from_key, [])
119-
120-
if len(date_froms) > 1:
121-
raise ParameterException(f"Search parameter {date_from_key} may have one value at most.")
122-
123-
try:
124-
date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date() \
125-
if len(date_froms) == 1 else date_from_default
126-
except ValueError:
127-
raise ParameterException(f"Search parameter {date_from_key} must be in format: YYYY-MM-DD")
128-
129-
# date.to
130-
date_tos = params.get(date_to_key, [])
131-
132-
if len(date_tos) > 1:
133-
raise ParameterException(f"Search parameter {date_to_key} may have one value at most.")
134-
135-
try:
136-
date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date() \
137-
if len(date_tos) == 1 else date_to_default
138-
except ValueError:
139-
raise ParameterException(f"Search parameter {date_to_key} must be in format: YYYY-MM-DD")
140-
141-
if date_from and date_to and date_from > date_to:
142-
raise ParameterException(f"Search parameter {date_from_key} must be before {date_to_key}")
143-
144-
# include
145-
includes = params.get(include_key, [])
146-
include = includes[0] if len(includes) > 0 else None
147-
148-
return SearchParams(patient_identifier, vaccine_types, date_from, date_to, include)
149-
150-
151188
def create_query_string(search_params: SearchParams) -> str:
152189
params = [
153190
(immunization_target_key, ",".join(map(quote, search_params.immunization_targets))),

‎backend/tests/test_parameter_parser.py‎

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
process_params,
1212
process_search_params,
1313
create_query_string,
14+
create_query_string,
15+
include_key,
1416
SearchParams,
1517
)
1618

@@ -106,7 +108,7 @@ def test_process_search_params_checks_patient_identifier_format(self):
106108
self.immunization_target_key: ["RSV"],
107109
}
108110
)
109-
self.assertIsNotNone(params)
111+
110112

111113
def test_process_search_params_whitelists_immunization_target(self):
112114
mock_redis_key = "RSV"
@@ -115,14 +117,18 @@ def test_process_search_params_whitelists_immunization_target(self):
115117
process_search_params(
116118
{
117119
self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"],
118-
self.immunization_target_key: ["not-a-code"],
120+
self.immunization_target_key: ["FLU", "COVID-19", "NOT-A-REAL-VALUE"],
119121
}
120122
)
121123
self.assertEqual(
122124
str(e.exception), f"immunization-target must be one or more of the following: {mock_redis_key}",
123125
f"Unexpected exception message: {str(e.exception)}"
124126
)
125127

128+
129+
def test_process_search_params_immunization_target(self):
130+
mock_redis_key = "RSV"
131+
self.mock_redis_client.hkeys.return_value = [mock_redis_key]
126132
params = process_search_params(
127133
{
128134
self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"],
@@ -132,6 +138,7 @@ def test_process_search_params_whitelists_immunization_target(self):
132138

133139
self.assertIsNotNone(params)
134140

141+
135142
def test_search_params_date_from_must_be_before_date_to(self):
136143
self.mock_redis_client.hkeys.return_value = ["RSV"]
137144
params = process_search_params(
@@ -210,3 +217,83 @@ def test_create_query_string_with_multiple_immunization_targets_comma_separated(
210217
expected = "-immunization.target=b,c&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7Ca"
211218

212219
self.assertEqual(expected, query_string)
220+
221+
def test_process_search_params_dedupes_immunization_targets_and_respects_include(self):
222+
"""Ensure duplicate immunization targets are deduped and include is preserved."""
223+
self.mock_redis_client.hkeys.return_value = ["RSV", "FLU"]
224+
225+
params = process_search_params(
226+
{
227+
self.patient_identifier_key: [
228+
"https://fhir.nhs.uk/Id/nhs-number|9000000009"
229+
],
230+
self.immunization_target_key: ["RSV", "RSV", "FLU"],
231+
"_include": ["immunization:patient"],
232+
}
233+
)
234+
235+
# immunization targets should be deduped and preserve valid entries
236+
self.assertIsInstance(params.immunization_targets, list)
237+
self.assertCountEqual(params.immunization_targets, ["RSV", "FLU"])
238+
239+
# include should be returned as provided
240+
self.assertEqual(params.include, "immunization:patient")
241+
242+
def test_process_search_params_aggregates_date_errors(self):
243+
"""When multiple date-related errors occur they should be returned together."""
244+
self.mock_redis_client.hkeys.return_value = ["RSV"]
245+
246+
with self.assertRaises(ParameterException) as e:
247+
process_search_params(
248+
{
249+
self.patient_identifier_key: [
250+
"https://fhir.nhs.uk/Id/nhs-number|9000000009"
251+
],
252+
self.immunization_target_key: ["RSV"],
253+
self.date_from_key: ["2021-01-01", "2021-01-02"], # too many values
254+
self.date_to_key: ["invalid-date"], # invalid format
255+
}
256+
)
257+
258+
expected = (
259+
f"Search parameter {self.date_from_key} may have one value at most.; "
260+
f"Search parameter {self.date_to_key} must be in format: YYYY-MM-DD"
261+
)
262+
self.assertEqual(str(e.exception), expected)
263+
264+
def test_process_search_params_invalid_nhs_number_is_rejected(self):
265+
"""If the NHS number fails mod11 check a ParameterException is raised."""
266+
# redis returns a valid vaccine type
267+
self.mock_redis_client.hkeys.return_value = ["RSV"]
268+
269+
with self.assertRaises(ParameterException) as e:
270+
process_search_params(
271+
{
272+
self.patient_identifier_key: [
273+
"https://fhir.nhs.uk/Id/nhs-number|1234567890" # invalid mod11
274+
],
275+
self.immunization_target_key: ["RSV"],
276+
}
277+
)
278+
279+
self.assertEqual(str(e.exception), f"Search parameter {self.patient_identifier_key} must be a valid NHS number.")
280+
281+
def test_process_search_params_invalid_include_value_is_rejected(self):
282+
"""_include may only be 'Immunization:patient' if provided."""
283+
self.mock_redis_client.hkeys.return_value = ["RSV"]
284+
285+
with self.assertRaises(ParameterException) as e:
286+
process_search_params(
287+
{
288+
self.patient_identifier_key: [
289+
"https://fhir.nhs.uk/Id/nhs-number|9000000009"
290+
],
291+
self.immunization_target_key: ["RSV"],
292+
"_include": ["Patient:practitioner"],
293+
}
294+
)
295+
296+
self.assertEqual(
297+
str(e.exception),
298+
f"Search parameter {include_key} may only be 'Immunization:patient' if provided.",
299+
)

0 commit comments

Comments
 (0)