Skip to content

Commit 9d3ea26

Browse files
Akol125Akol125
committed
refactor validation for lists
1 parent f44002a commit 9d3ea26

File tree

8 files changed

+89
-29
lines changed

8 files changed

+89
-29
lines changed

‎lambdas/shared/src/common/validator/expression_checker.py‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,8 @@ def validation_for_list(self, expression_rule: str, field_name: str, field_value
161161
"""
162162
rules = expression_rule_per_field(expression_rule) if expression_rule else {}
163163
defined_length: Optional[int] = rules.get("defined_length", None)
164-
max_length: Optional[int] = rules.get("max_length", None)
164+
array_max_length: Optional[int] = rules.get("array_max_length", None)
165165
elements_are_strings: bool = rules.get("elements_are_strings", False)
166-
string_element_max_length: Optional[int] = rules.get("string_element_max_length", None)
167166
elements_are_dicts: bool = rules.get("elements_are_dicts", False)
168167

169168
try:
@@ -177,25 +176,30 @@ def validation_for_list(self, expression_rule: str, field_name: str, field_value
177176
if len(field_value) == 0:
178177
raise ValueError(f"{field_name} must be a non-empty array")
179178

180-
if max_length is not None and len(field_value) > max_length:
181-
raise ValueError(f"{field_name} must be an array of maximum length {max_length}")
179+
if array_max_length is not None and len(field_value) > array_max_length:
180+
raise ValueError(f"{field_name} must be an array of maximum length {array_max_length}")
182181

183182
if elements_are_strings:
184183
for idx, element in enumerate(field_value):
185-
self._validate_for_string_values.for_string(
186-
element, f"{field_name}[{idx}]", max_length=string_element_max_length
187-
)
184+
error_report = self.validation_for_string_values(expression_rule, f"{field_name}[{idx}]", element)
185+
if error_report is not None:
186+
return error_report
188187

189188
if elements_are_dicts:
190189
for element in field_value:
191190
if not isinstance(element, dict):
192191
raise TypeError(f"{field_name} must be an array of objects")
193192
if len(element) == 0:
194193
raise ValueError(f"{field_name} must be an array of non-empty objects")
194+
except (TypeError, ValueError) as e:
195+
code = ExceptionLevels.RECORD_CHECK_FAILED
196+
message = MESSAGES[ExceptionLevels.RECORD_CHECK_FAILED]
197+
details = str(e)
198+
return ErrorReport(code, message, None, field_name, details)
195199
except Exception as e:
196200
if self.report_unexpected_exception:
197201
message = MESSAGES[ExceptionLevels.UNEXPECTED_EXCEPTION] % (e.__class__.__name__, e)
198-
return ErrorReport(ExceptionLevels.UNEXPECTED_EXCEPTION, message, field_name)
202+
return ErrorReport(ExceptionLevels.UNEXPECTED_EXCEPTION, message, None, field_name, "")
199203

200204
def validation_for_date_time(
201205
self, expression_rule: str, field_name: str, field_value: str, row: dict, strict_timezone: bool = True

‎lambdas/shared/src/common/validator/expression_rule.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ def expression_rule_per_field(expression_type: str) -> dict:
66
case "NHS_NUMBER":
77
return {"defined_length": Constants.NHS_NUMBER_LENGTH, "spaces_allowed": False}
88
case "PERSON_NAME":
9-
return {"max_length": Constants.PERSON_NAME_ELEMENT_MAX_LENGTH}
10-
case "PERSON_SURNAME":
119
return {
1210
"elements_are_strings": True,
13-
"max_length": 5,
14-
"string_element_max_length": Constants.PERSON_NAME_ELEMENT_MAX_LENGTH,
11+
"array_max_length": 5,
12+
"max_length": Constants.PERSON_NAME_ELEMENT_MAX_LENGTH,
1513
}
14+
case "PERSON_SURNAME":
15+
return {"max_length": Constants.PERSON_NAME_ELEMENT_MAX_LENGTH}
1616
case "GENDER":
1717
return {"predefined_values": Constants.GENDERS}
1818
case _:

‎lambdas/shared/tests/test_common/validator/test_csv_line_parser.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,20 @@ def test_extra_values_ignored(self):
2222
Ignore values that do not have a corresponding key
2323
"""
2424
csv_parsers = CSVLineParser()
25-
csv_parsers.parse_csv_line({"NHS_NUMBER": "9000000009", "PERSON_FORENAME": "Alex", "": "Trent"})
25+
csv_parsers.parse_csv_line({"NHS_NUMBER": "9000000009", "PERSON_FORENAME": ["Alex"], "": "Trent"})
2626
self.assertEqual(
2727
csv_parsers.csv_file_data,
28-
{"NHS_NUMBER": "9000000009", "PERSON_FORENAME": "Alex", "": "Trent"},
28+
{"NHS_NUMBER": "9000000009", "PERSON_FORENAME": ["Alex"], "": "Trent"},
2929
)
30-
self.assertEqual(csv_parsers.get_key_value("PERSON_FORENAME"), "Alex")
30+
self.assertEqual(csv_parsers.get_key_value("PERSON_FORENAME"), ["Alex"])
3131

3232
def test_fewer_values_than_keys(self):
3333
"""
3434
Test that fewer values (rows) than keys (columns/headers)
3535
raises an error when accessing key without value
3636
"""
3737
csv_parsers = CSVLineParser()
38-
csv_parsers.parse_csv_line({"NHS_NUMBER": "9000000009", "PERSON_FORENAME": "Alex"})
38+
csv_parsers.parse_csv_line({"NHS_NUMBER": "9000000009", "PERSON_FORENAME": ["Alex"]})
3939
self.assertIn("NHS_NUMBER", csv_parsers.csv_file_data)
4040
self.assertIn("PERSON_FORENAME", csv_parsers.csv_file_data)
4141
self.assertNotIn("PERSON_SURNAME", csv_parsers.csv_file_data)
@@ -46,7 +46,7 @@ def test_get_missing_key_raises(self):
4646
"""
4747
Test that accessing a non-existent key raises KeyError"""
4848
csv_parsers = CSVLineParser()
49-
csv_parsers.parse_csv_line({"NHS_NUMBER": "9000000009", "PERSON_FORENAME": "Alex"})
49+
csv_parsers.parse_csv_line({"NHS_NUMBER": "9000000009", "PERSON_FORENAME": ["Alex"]})
5050
with self.assertRaises(KeyError):
5151
_ = csv_parsers.get_key_value("VACCINE_TYPE")
5252

‎lambdas/shared/tests/test_common/validator/test_expression_checker.py‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ def test_string_valid_and_invalid(self):
3232
"9876543210",
3333
)
3434
)
35+
self.assertIsNone(
36+
checker.validate_expression(
37+
"STRING",
38+
"NHS_NUMBER",
39+
"contained|#:Patient|identifier|#:https://fhir.nhs.uk/Id/nhs-number|value",
40+
"9876543210",
41+
)
42+
)
3543
# Empty should fail NHS number string rule
3644
self.assertIsInstance(
3745
checker.validate_expression(
@@ -40,18 +48,38 @@ def test_string_valid_and_invalid(self):
4048
ErrorReport,
4149
)
4250

43-
# LIST
44-
def test_list_valid_and_invalid(self):
45-
checker = self.make_checker()
46-
self.assertIsNone(checker.validate_expression("LIST", "PERSON_NAME", "PERSON_NAME", ["Alice"]))
47-
self.assertIsInstance(checker.validate_expression("LIST", "PERSON_NAME", "PERSON_NAME", []), ErrorReport)
48-
self.assertIsInstance(checker.validate_expression("LIST", "", "PERSON_NAME", "Alice"), ErrorReport)
51+
# VALID PERSON_SURNAME STRING
52+
self.assertIsNone(
53+
checker.validate_expression(
54+
"STRING", "PERSON_SURNAME", "contained|#:Patient|name|#:official|family", "Smith"
55+
)
56+
)
57+
self.assertIsNone(checker.validate_expression("STRING", "PERSON_SURNAME", "PERSON_SURNAME", "Taylor"))
58+
# INVALID PERSON_SURNAME STRING (too long)
59+
self.assertIsInstance(
60+
checker.validate_expression(
61+
"STRING", "PERSON_SURNAME", "contained|#:Patient|name|#:official|family", "Stan" * 51
62+
),
63+
ErrorReport,
64+
)
4965

50-
# DATE
51-
def test_date_valid_and_invalid(self):
66+
# LIST PERSON_FORENAME
67+
def test_list_valid_and_invalid(self):
5268
checker = self.make_checker()
53-
self.assertIsNone(checker.validate_expression("DATE", "", "date_field", "2025-01-01"))
54-
self.assertIsInstance(checker.validate_expression("DATE", "", "date_field", "2025-13-01"), ErrorReport)
69+
self.assertIsNone(checker.validate_expression("LIST", "PERSON_NAME", "PERSON_FORENAME", ["Alice"]))
70+
self.assertIsNone(
71+
checker.validate_expression(
72+
"LIST", "PERSON_NAME", "contained|#:Patient|name|#:official|given|0", ["Bethany"]
73+
)
74+
)
75+
self.assertIsInstance(checker.validate_expression("LIST", "PERSON_NAME", "PERSON_FORENAME", []), ErrorReport)
76+
self.assertIsInstance(checker.validate_expression("LIST", "", "PERSON_FORENAME", "Alice"), ErrorReport)
77+
78+
# # DATE
79+
# def test_date_valid_and_invalid(self):
80+
# checker = self.make_checker()
81+
# self.assertIsNone(checker.validate_expression("DATE", "", "contained|#:Patient|name|#:official|given|0", "2025-01-01"))
82+
# self.assertIsInstance(checker.validate_expression("DATE", "", "date_field", "2025-13-01"), ErrorReport)
5583

5684

5785
# # DATETIME

‎lambdas/shared/tests/test_common/validator/test_schemas/test_schema.json‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,37 @@
2424
"fieldNumber": 2,
2525
"errorLevel": 0,
2626
"expression": {
27-
"expressionName": "Person Forname List Check",
27+
"expressionName": "Person Forename List Check",
2828
"expressionType": "LIST",
2929
"expressionRule": "PERSON_NAME"
3030
},
3131
"errorGroup": "completeness"
32+
},
33+
{
34+
"expressionId": "01K5EGR0C7RRG9F6FVHJ8HE4QX",
35+
"fieldNameFHIR": "contained|#:Patient|name|#:official|family",
36+
"fieldNameFlat": "PERSON_SURNAME",
37+
"fieldNumber": 3,
38+
"errorLevel": 0,
39+
"expression": {
40+
"expressionName": "Person Surname String Check",
41+
"expressionType": "STRING",
42+
"expressionRule": "PERSON_SURNAME"
43+
},
44+
"errorGroup": "completeness"
45+
},
46+
{
47+
"expressionId": "01K8RW1DF635S4FRZ1WF9GDS1T",
48+
"fieldNameFHIR": "contained|#:Patient|birthDate",
49+
"fieldNameFlat": "PERSON_DOB",
50+
"fieldNumber": 4,
51+
"errorLevel": 1,
52+
"expression": {
53+
"expressionName": "Date of Birth Not Empty Check",
54+
"expressionType": "DATE",
55+
"expressionRule": ""
56+
},
57+
"errorGroup": "consistency"
3258
}
3359
]
3460
}

‎lambdas/shared/tests/test_common/validator/test_validation_csv_row.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ def setUp(self):
2424

2525
def test_run_validation_on_valid_csv_row(self):
2626
error_list = self.validator.validate_csv_row(CSV_VALUES, True, True, True)
27+
print(f"Run Validation Errors for valid CSV row: {error_list}")
2728
self.assertEqual(error_list, [])
2829

2930
def test_run_validation_on_invalid_csv_row(self):

‎lambdas/shared/tests/test_common/validator/test_validator.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ def test_validate_csv(self):
2727

2828
self.assertIsInstance(result, list)
2929
self.assertTrue(all(isinstance(err, ErrorReport) for err in result))
30+
print(f"CSV Validation Errors: {result}")
3031
self.assertEqual(len(result), 0, "Expected no validation errors for valid CSV row")
3132

3233
def test_has_validation_failed_detects_critical_error(self):

‎lambdas/shared/tests/test_common/validator/testing_utils/constants.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"NHS_NUMBER": "9000000009",
1515
"PERSON_FORENAME": ["JOHN"],
1616
"PERSON_SURNAME": "DOE",
17-
"PERSON_DOB": "19800101",
17+
"PERSON_DOB": "1980-01-01",
1818
"PERSON_GENDER_CODE": "M",
1919
"PERSON_POSTCODE": "AB12 3CD",
2020
"DATE_AND_TIME": "20250306T13281701",

0 commit comments

Comments
 (0)