From a6477026fea3ca1caec7bad724f1005fe8286932 Mon Sep 17 00:00:00 2001 From: eesa456 Date: Wed, 23 Apr 2025 15:46:44 +0100 Subject: [PATCH 01/12] NRL-1285 update parent model with validator for empty fields --- .../tests/test_create_document_reference.py | 44 +++++++++++++++ layer/nrlf/core/parent_model.py | 56 ++++++++++++++++++- .../createDocumentReference-failure.feature | 32 +++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index a9c85d796..08b482368 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -384,6 +384,50 @@ def test_create_document_reference_invalid_body(): } +def test_create_document_reference_empty_fields_in_body(): + doc_ref = load_document_reference("Y05868-736253002-Valid") + doc_ref.author = [] + doc_ref.context = {"practiceSetting": {}, "sourcePatientInfo": None} + doc_ref.category = [{"coding": [{"system": "", "code": None}]}] + doc_ref.text = "" + + event = create_test_api_gateway_event( + headers=create_headers(), + body=doc_ref.model_dump_json(exclude_none=True), + ) + result = handler(event, create_mock_context()) + body = result.pop("body") + + assert result == { + "statusCode": "400", + "headers": default_response_headers(), + "isBase64Encoded": False, + } + + parsed_body = json.loads(body) + + assert parsed_body == { + "resourceType": "OperationOutcome", + "issue": [ + { + "severity": "error", + "code": "invalid", + "details": { + "coding": [ + { + "code": "MESSAGE_NOT_WELL_FORMED", + "display": "Message not well formed", + "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", + } + ], + }, + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: text, author, context.sourcePatientInfo, category[0].coding[0].system, category[0].coding[0].code)", + "expression": ["root"], + } + ], + } + + def test_create_document_reference_invalid_resource(): doc_ref = load_document_reference("Y05868-736253002-Valid") doc_ref.custodian = None diff --git a/layer/nrlf/core/parent_model.py b/layer/nrlf/core/parent_model.py index 3c18be72c..71b084030 100644 --- a/layer/nrlf/core/parent_model.py +++ b/layer/nrlf/core/parent_model.py @@ -1,6 +1,6 @@ from typing import Annotated, List, Optional -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, model_validator class ParentCoding(BaseModel): @@ -81,6 +81,60 @@ class ParentExtension(BaseModel): class Parent(BaseModel): + @model_validator(mode="before") + @classmethod + def validate_empty_fields(cls, values): + """ + Iteratively check every field in the model for emptiness. + If a field is empty, add it to the error list with its full location. + """ + stack = [(None, values)] + empty_fields = [] + + while stack: + path, current_value = stack.pop() + + if isinstance(current_value, dict): + for key, value in current_value.items(): + full_path = f"{path}.{key}" if path else key + if ( + value is None + or value == "" + or (isinstance(value, list) and not value) + ): + empty_fields.append(full_path) + else: + stack.append((full_path, value)) + + elif isinstance(current_value, list): + for index, item in enumerate(current_value): + full_path = f"{path}[{index}]" if path else f"[{index}]" + if ( + item is None + or item == "" + or (isinstance(item, dict) and not item) + ): + empty_fields.append(full_path) + else: + stack.append((full_path, item)) + + elif isinstance(current_value, BaseModel): + nested_values = current_value.model_dump(exclude_none=True) + for nested_field, nested_value in nested_values.items(): + full_path = f"{path}.{nested_field}" if path else nested_field + stack.append((full_path, nested_value)) + + else: + if current_value is None or current_value == "": + empty_fields.append(path) + + if empty_fields: + raise ValueError( + f"The following fields are empty: {', '.join(empty_fields)}" + ) + + return values + model_config = ConfigDict(regex_engine="python-re", extra="forbid") extension: Annotated[ Optional[List[ParentExtension]], diff --git a/tests/features/producer/createDocumentReference-failure.feature b/tests/features/producer/createDocumentReference-failure.feature index 1510543a0..6540aa373 100644 --- a/tests/features/producer/createDocumentReference-failure.feature +++ b/tests/features/producer/createDocumentReference-failure.feature @@ -1014,3 +1014,35 @@ Feature: Producer - createDocumentReference - Failure Scenarios ] } """ + + Scenario: Reject DocumentReference with empty non-mandatory field (author) + Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API + And the organisation 'TSTCUS' is authorised to access pointer types: + | system | value | + | http://snomed.info/sct | 736253002 | + When producer 'TSTCUS' requests creation of a DocumentReference with default test values except 'author' is: + """ + "author": [] + """ + Then the response status code is 400 + And the response is an OperationOutcome with 1 issue + And the OperationOutcome contains the issue: + """ + { + "severity": "error", + "code": "invalid", + "details": { + "coding": [ + { + "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", + "code": "MESSAGE_NOT_WELL_FORMED", + "display": "Message not well formed" + } + ] + }, + "diagnostics": "Request body could not be parsed (author: Field is an empty list)", + "expression": [ + "author" + ] + } + """ From fafff0e6550339dc195caf233b83823587b2a67b Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 24 Apr 2025 03:43:15 +0100 Subject: [PATCH 02/12] NRL-1285 tests and update to validate certain classes --- .../tests/test_create_document_reference.py | 37 ++------ .../tests/test_update_document_reference.py | 13 +-- .../tests/test_upsert_document_reference.py | 34 ++----- layer/nrlf/core/parent_model.py | 23 +++++ layer/nrlf/core/tests/test_pydantic_errors.py | 95 ------------------- .../createDocumentReference-failure.feature | 6 +- 6 files changed, 47 insertions(+), 161 deletions(-) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index 08b482368..309f9744b 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -387,7 +387,7 @@ def test_create_document_reference_invalid_body(): def test_create_document_reference_empty_fields_in_body(): doc_ref = load_document_reference("Y05868-736253002-Valid") doc_ref.author = [] - doc_ref.context = {"practiceSetting": {}, "sourcePatientInfo": None} + doc_ref.custodian = {"identifier": {}, "reference": None} doc_ref.category = [{"coding": [{"system": "", "code": None}]}] doc_ref.text = "" @@ -421,7 +421,7 @@ def test_create_document_reference_empty_fields_in_body(): } ], }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: text, author, context.sourcePatientInfo, category[0].coding[0].system, category[0].coding[0].code)", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: text, author, custodian.reference, category[0].coding[0].system, category[0].coding[0].code)", "expression": ["root"], } ], @@ -825,9 +825,7 @@ def test_create_document_reference_cannot_set_status_to_not_current(repository): def test_create_document_reference_no_relatesto_target(): doc_ref = load_document_reference("Y05868-736253002-Valid") doc_ref.relatesTo = [ - DocumentReferenceRelatesTo( - code="transforms", target=Reference(reference=None, identifier=None) - ) + DocumentReferenceRelatesTo(code="transforms", target=Reference()) ] event = create_test_api_gateway_event( @@ -873,9 +871,7 @@ def test_create_document_reference_invalid_relatesto_target_producer_id(): doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="X26-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="X26-99999-99999-999999")), ) ] @@ -925,7 +921,6 @@ def test_create_document_reference_invalid_relatesto_not_exists(repository): DocumentReferenceRelatesTo( code="transforms", target=Reference( - reference=None, identifier=Identifier(value="Y05868-123456-123456-123456"), ), ) @@ -986,9 +981,7 @@ def test_create_document_reference_invalid_relatesto_nhs_number( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] @@ -1048,9 +1041,7 @@ def test_create_document_reference_invalid_relatesto_type( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] @@ -1264,9 +1255,7 @@ def test_create_document_reference_supersede_deletes_old_pointers_replace( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="replaces", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] @@ -1325,9 +1314,7 @@ def test_create_document_reference_supersede_succeeds_with_toggle( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="replaces", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-000000") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")), ) ] @@ -1385,9 +1372,7 @@ def test_create_document_reference_supersede_fails_without_toggle( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="replaces", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-000000") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")), ) ] @@ -1444,9 +1429,7 @@ def test_create_document_reference_create_relatesto_not_replaces( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] diff --git a/api/producer/updateDocumentReference/tests/test_update_document_reference.py b/api/producer/updateDocumentReference/tests/test_update_document_reference.py index d2f06b669..d5e5406a2 100644 --- a/api/producer/updateDocumentReference/tests/test_update_document_reference.py +++ b/api/producer/updateDocumentReference/tests/test_update_document_reference.py @@ -40,7 +40,7 @@ def test_update_document_reference_happy_path(repository: DocumentPointerReposit event = create_test_api_gateway_event( headers=create_headers(), path_parameters={"id": "Y05868-99999-99999-999999"}, - body=doc_ref.model_dump_json(), + body=doc_ref.model_dump_json(exclude_none=True), ) result = handler(event, create_mock_context()) @@ -109,7 +109,7 @@ def test_update_document_reference_happy_path_with_ssp( event = create_test_api_gateway_event( headers=create_headers(), path_parameters={"id": "Y05868-99999-99999-999999"}, - body=doc_ref.model_dump_json(), + body=doc_ref.model_dump_json(exclude_none=True), ) result = handler(event, create_mock_context()) @@ -580,18 +580,13 @@ def test_update_document_reference_immutable_fields(repository): repository.create(doc_pointer) doc_ref.type = CodeableConcept( - id=None, coding=[ Coding( - id=None, system="http://snomed.info/sct", - version=None, code="861421000000109", display="End of life care coordination summary", - userSelected=None, ) ], - text=None, ) event = create_test_api_gateway_event( @@ -861,7 +856,7 @@ def test_update_document_reference_with_meta_lastupdated_ignored( event = create_test_api_gateway_event( headers=create_headers(), path_parameters={"id": "Y05868-99999-99999-999999"}, - body=doc_ref.model_dump_json(), + body=doc_ref.model_dump_json(exclude_none=True), ) result = handler(event, create_mock_context()) @@ -931,7 +926,7 @@ def test_update_document_reference_with_invalid_date_ignored( event = create_test_api_gateway_event( headers=create_headers(), path_parameters={"id": "Y05868-99999-99999-999999"}, - body=doc_ref.model_dump_json(), + body=doc_ref.model_dump_json(exclude_none=True), ) result = handler(event, create_mock_context()) diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 35a087f55..9d676d835 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -748,9 +748,7 @@ def test_upsert_document_reference_pointer_type_not_allowed( def test_upsert_document_reference_no_relatesto_target(): doc_ref = load_document_reference("Y05868-736253002-Valid") doc_ref.relatesTo = [ - DocumentReferenceRelatesTo( - code="transforms", target=Reference(reference=None, identifier=None) - ) + DocumentReferenceRelatesTo(code="transforms", target=Reference()) ] event = create_test_api_gateway_event( @@ -796,9 +794,7 @@ def test_upsert_document_reference_invalid_relatesto_target_producer_id(): doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="X26-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="X26-99999-99999-999999")), ) ] @@ -848,7 +844,6 @@ def test_upsert_document_reference_invalid_relatesto_not_exists(repository): DocumentReferenceRelatesTo( code="transforms", target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999"), ), ) @@ -902,7 +897,6 @@ def test_upsert_document_reference_invalid_relatesto_not_exists_still_creates_wi DocumentReferenceRelatesTo( code="transforms", target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999"), ), ) @@ -967,9 +961,7 @@ def test_upsert_document_reference_invalid_relatesto_nhs_number( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] @@ -1029,9 +1021,7 @@ def test_upsert_document_reference_invalid_relatesto_type( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] @@ -1244,9 +1234,7 @@ def test_upsert_document_reference_supersede_deletes_old_pointers_replace( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="replaces", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] @@ -1304,9 +1292,7 @@ def test_upsert_document_reference_supersede_succeeds_with_toggle( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="replaces", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-000000") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")), ) ] @@ -1364,9 +1350,7 @@ def test_upsert_document_reference_supersede_fails_without_toggle( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="replaces", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-000000") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")), ) ] @@ -1422,9 +1406,7 @@ def test_upsert_document_reference_create_relatesto_not_replaces( doc_ref.relatesTo = [ DocumentReferenceRelatesTo( code="transforms", - target=Reference( - reference=None, identifier=Identifier(value="Y05868-99999-99999-999999") - ), + target=Reference(identifier=Identifier(value="Y05868-99999-99999-999999")), ) ] diff --git a/layer/nrlf/core/parent_model.py b/layer/nrlf/core/parent_model.py index 71b084030..2e5d3f966 100644 --- a/layer/nrlf/core/parent_model.py +++ b/layer/nrlf/core/parent_model.py @@ -88,6 +88,29 @@ def validate_empty_fields(cls, values): Iteratively check every field in the model for emptiness. If a field is empty, add it to the error list with its full location. """ + allowed_classes = [ + "DocumentReference", + "Meta", + "Narrative", + "Identifier", + "NRLCodeableConcept", + "NRLCoding", + "Reference", + "DocumentReferenceRelatesTo", + "CodeableConcept", + "Coding", + "DocumentReferenceContent", + "Attachment", + "NRLFormatCode", + "ContentStabilityExtension", + "ContentStabilityExtensionValueCodeableConcept", + "ContentStabilityExtensionCoding", + "DocumentReferenceContext", + "Period", + ] + if cls.__name__ not in allowed_classes: + return values + stack = [(None, values)] empty_fields = [] diff --git a/layer/nrlf/core/tests/test_pydantic_errors.py b/layer/nrlf/core/tests/test_pydantic_errors.py index 033961446..aae0da920 100644 --- a/layer/nrlf/core/tests/test_pydantic_errors.py +++ b/layer/nrlf/core/tests/test_pydantic_errors.py @@ -242,37 +242,6 @@ def test_validate_content_invalid_content_stability_url(): } -def test_validate_content_empty_content_stability_coding(): - validator = DocumentReferenceValidator() - document_ref_data = load_document_reference_json("Y05868-736253002-Valid") - - # Set an empty coding list for contentStability extension - document_ref_data["content"][0]["extension"][0]["valueCodeableConcept"][ - "coding" - ] = [] - - with pytest.raises(ParseError) as error: - validator.validate(document_ref_data) - - exc = error.value - assert len(exc.issues) == 1 - assert exc.issues[0].model_dump(exclude_none=True) == { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_RESOURCE", - "display": "Invalid validation of resource", - } - ] - }, - "diagnostics": "Failed to parse DocumentReference resource (content[0].extension[0].valueCodeableConcept.coding: List should have at least 1 item after validation, not 0. See ValueSet: https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability)", - "expression": ["content[0].extension[0].valueCodeableConcept.coding"], - } - - def test_validate_content_missing_content_stability_coding(): validator = DocumentReferenceValidator() document_ref_data = load_document_reference_json("Y05868-736253002-Valid") @@ -350,70 +319,6 @@ def test_validate_multiple_codings(): } -def test_validate_missing_coding(): - validator = DocumentReferenceValidator() - document_ref_data = load_document_reference_json("Y05868-736253002-Valid") - - document_ref_data["category"][0] = {"coding": []} - - with pytest.raises(ParseError) as error: - validator.validate(document_ref_data) - - exc = error.value - assert len(exc.issues) == 1 - assert exc.issues[0].model_dump(exclude_none=True) == { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_RESOURCE", - "display": "Invalid validation of resource", - } - ] - }, - "diagnostics": "Failed to parse DocumentReference resource (category[0].coding: List should have at least 1 item after validation, not 0)", - "expression": ["category[0].coding"], - } - - -def test_validate_empty_strings(): - validator = DocumentReferenceValidator() - document_ref_data = load_document_reference_json("Y05868-736253002-Valid") - - document_ref_data["category"][0] = { - "coding": [ - { - "system": SNOMED_SYSTEM_URL, - "code": "734163000", - "display": "", - } - ] - } - - with pytest.raises(ParseError) as error: - validator.validate(document_ref_data) - - exc = error.value - assert len(exc.issues) == 1 - assert exc.issues[0].model_dump(exclude_none=True) == { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_RESOURCE", - "display": "Invalid validation of resource", - } - ] - }, - "diagnostics": "Failed to parse DocumentReference resource (category[0].coding[0].display: String should match pattern '[\\S]+[ \\r\\n\\t\\S]*')", - "expression": ["category[0].coding[0].display"], - } - - def test_validate_whitespace_strings(): validator = DocumentReferenceValidator() document_ref_data = load_document_reference_json("Y05868-736253002-Valid") diff --git a/tests/features/producer/createDocumentReference-failure.feature b/tests/features/producer/createDocumentReference-failure.feature index 6540aa373..ec402cbdf 100644 --- a/tests/features/producer/createDocumentReference-failure.feature +++ b/tests/features/producer/createDocumentReference-failure.feature @@ -1040,9 +1040,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (author: Field is an empty list)", - "expression": [ - "author" - ] + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: author)", + "expression": ["root"] } """ From 961b2508136e4db2372c445f5601e7e6ced2823c Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 24 Apr 2025 11:12:34 +0100 Subject: [PATCH 03/12] NRL-1285 update int tests --- .../producer/createDocumentReference-failure.feature | 12 ++++++------ .../producer/updateDocumentReference-failure.feature | 4 ++-- .../producer/upsertDocumentReference-failure.feature | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/features/producer/createDocumentReference-failure.feature b/tests/features/producer/createDocumentReference-failure.feature index ec402cbdf..864e38bf2 100644 --- a/tests/features/producer/createDocumentReference-failure.feature +++ b/tests/features/producer/createDocumentReference-failure.feature @@ -744,9 +744,9 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (content: List should have at least 1 item after validation, not 0)", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content)", "expression": [ - "content" + "root" ] } """ @@ -802,9 +802,9 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (content[0].attachment.contentType: String should match pattern '[^\\s]+(\\s[^\\s]+)*')", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content[0].attachment.contentType)", "expression": [ - "content[0].attachment.contentType" + "root" ] } """ @@ -1008,9 +1008,9 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (context.practiceSetting.coding[0].display: String should match pattern '[\\S]+[ \\r\\n\\t\\S]*')", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: context.practiceSetting.coding[0].display)", "expression": [ - "context.practiceSetting.coding[0].display" + "root" ] } """ diff --git a/tests/features/producer/updateDocumentReference-failure.feature b/tests/features/producer/updateDocumentReference-failure.feature index 4dd5ea87c..52ed4e036 100644 --- a/tests/features/producer/updateDocumentReference-failure.feature +++ b/tests/features/producer/updateDocumentReference-failure.feature @@ -87,9 +87,9 @@ Feature: Producer - updateDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (content: List should have at least 1 item after validation, not 0)", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content)", "expression": [ - "content" + "root" ] } """ diff --git a/tests/features/producer/upsertDocumentReference-failure.feature b/tests/features/producer/upsertDocumentReference-failure.feature index b86cdf60c..c08598ccc 100644 --- a/tests/features/producer/upsertDocumentReference-failure.feature +++ b/tests/features/producer/upsertDocumentReference-failure.feature @@ -265,9 +265,9 @@ Feature: Producer - upsertDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (content: List should have at least 1 item after validation, not 0)", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content)", "expression": [ - "content" + "root" ] } """ @@ -323,9 +323,9 @@ Feature: Producer - upsertDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (content[0].attachment.contentType: String should match pattern '[^\\s]+(\\s[^\\s]+)*')", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content[0].attachment.contentType)", "expression": [ - "content[0].attachment.contentType" + "root" ] } """ From a2f999e30604663edb3b03501132a8fca6f6dfd4 Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 24 Apr 2025 12:46:28 +0100 Subject: [PATCH 04/12] NRL-1285 fix update test --- .../features/producer/updateDocumentReference-failure.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/features/producer/updateDocumentReference-failure.feature b/tests/features/producer/updateDocumentReference-failure.feature index 52ed4e036..f9065ac67 100644 --- a/tests/features/producer/updateDocumentReference-failure.feature +++ b/tests/features/producer/updateDocumentReference-failure.feature @@ -158,9 +158,9 @@ Feature: Producer - updateDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (content[0].attachment.contentType: String should match pattern '[^\\s]+(\\s[^\\s]+)*')", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content[0].attachment.contentType)", "expression": [ - "content[0].attachment.contentType" + "root" ] } """ From 69ae091b6a7c3e3d919377491fa97f695169098f Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 24 Apr 2025 13:32:22 +0100 Subject: [PATCH 05/12] NRL-1285 fix smoke tests --- tests/smoke/scenarios/1dsync_upsert_delete.py | 4 +++- tests/smoke/setup.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/smoke/scenarios/1dsync_upsert_delete.py b/tests/smoke/scenarios/1dsync_upsert_delete.py index 9e9fecb25..c314dd241 100644 --- a/tests/smoke/scenarios/1dsync_upsert_delete.py +++ b/tests/smoke/scenarios/1dsync_upsert_delete.py @@ -39,7 +39,9 @@ def test_1dsync_upsert_delete( test_docref.id = ( f"{test_ods_code}-smoketest_1dsync_upsert_delete_pointer_{attempts}" ) - upsert_response = producer_client_1dsync.upsert(test_docref.model_dump()) + upsert_response = producer_client_1dsync.upsert( + test_docref.model_dump(exclude_none=True) + ) assert upsert_response.ok assert upsert_response.headers["Location"].split("/")[-1] == test_docref.id finally: diff --git a/tests/smoke/setup.py b/tests/smoke/setup.py index 9bceec028..0979c3e01 100644 --- a/tests/smoke/setup.py +++ b/tests/smoke/setup.py @@ -141,7 +141,7 @@ def upsert_test_pointer( ) -> DocumentReference: docref.id = id - create_response = producer_client.upsert(docref.model_dump()) + create_response = producer_client.upsert(docref.model_dump(exclude_none=True)) if not create_response.ok: raise ValueError(f"Failed to create test pointer: {create_response.text}") From f86cb7d64aa5b6efb841a1b3ba1fb6a4c5012291 Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 24 Apr 2025 13:42:49 +0100 Subject: [PATCH 06/12] NRL-1285 update crud smoke test --- tests/smoke/scenarios/producer_crud.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/smoke/scenarios/producer_crud.py b/tests/smoke/scenarios/producer_crud.py index 24fcdadf1..8f4e5e578 100644 --- a/tests/smoke/scenarios/producer_crud.py +++ b/tests/smoke/scenarios/producer_crud.py @@ -18,7 +18,9 @@ def test_producer_crud( try: # Create - create_response = producer_client.create(test_docref.model_dump()) + create_response = producer_client.create( + test_docref.model_dump(exclude_none=True) + ) assert create_response.ok created_id = create_response.headers["Location"].split("/")[-1] @@ -29,7 +31,7 @@ def test_producer_crud( # Update updated_docref = { - **test_docref.model_dump(), + **test_docref.model_dump(exclude_none=True), "id": created_id, } updated_docref["content"][0]["attachment"][ From 8da54c4e9607e663296276103ff1ab39d2f09bbf Mon Sep 17 00:00:00 2001 From: eesa456 Date: Fri, 25 Apr 2025 12:10:38 +0100 Subject: [PATCH 07/12] NRL-1285 check if dict is empty --- .../tests/test_create_document_reference.py | 12 +-- .../tests/test_upsert_document_reference.py | 10 +-- layer/nrlf/core/parent_model.py | 7 +- layer/nrlf/core/tests/test_pydantic_errors.py | 31 ------- layer/nrlf/core/tests/test_validators.py | 83 ------------------- 5 files changed, 15 insertions(+), 128 deletions(-) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index 309f9744b..4fe10c800 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -421,7 +421,7 @@ def test_create_document_reference_empty_fields_in_body(): } ], }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: text, author, custodian.reference, category[0].coding[0].system, category[0].coding[0].code)", + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: text, author, custodian.reference, custodian.identifier, category[0].coding[0].system, category[0].coding[0].code)", "expression": ["root"], } ], @@ -853,14 +853,14 @@ def test_create_document_reference_no_relatesto_target(): "details": { "coding": [ { - "code": "BAD_REQUEST", - "display": "Bad request", + "code": "MESSAGE_NOT_WELL_FORMED", + "display": "Message not well formed", "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", } - ] + ], }, - "diagnostics": "No identifier value provided for relatesTo target", - "expression": ["relatesTo[0].target.identifier.value"], + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: relatesTo[0].target)", + "expression": ["root"], } ], } diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 9d676d835..3618e5624 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -776,14 +776,14 @@ def test_upsert_document_reference_no_relatesto_target(): "details": { "coding": [ { - "code": "BAD_REQUEST", - "display": "Bad request", + "code": "MESSAGE_NOT_WELL_FORMED", + "display": "Message not well formed", "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", } - ] + ], }, - "diagnostics": "No identifier value provided for relatesTo target", - "expression": ["relatesTo[0].target.identifier.value"], + "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: relatesTo[0].target)", + "expression": ["root"], } ], } diff --git a/layer/nrlf/core/parent_model.py b/layer/nrlf/core/parent_model.py index 2e5d3f966..fc1c2cb2b 100644 --- a/layer/nrlf/core/parent_model.py +++ b/layer/nrlf/core/parent_model.py @@ -90,7 +90,6 @@ def validate_empty_fields(cls, values): """ allowed_classes = [ "DocumentReference", - "Meta", "Narrative", "Identifier", "NRLCodeableConcept", @@ -108,7 +107,7 @@ def validate_empty_fields(cls, values): "DocumentReferenceContext", "Period", ] - if cls.__name__ not in allowed_classes: + if cls.__name__ not in allowed_classes or not values: return values stack = [(None, values)] @@ -128,6 +127,8 @@ def validate_empty_fields(cls, values): empty_fields.append(full_path) else: stack.append((full_path, value)) + if not current_value: + empty_fields.append(path) elif isinstance(current_value, list): for index, item in enumerate(current_value): @@ -141,7 +142,7 @@ def validate_empty_fields(cls, values): else: stack.append((full_path, item)) - elif isinstance(current_value, BaseModel): + elif isinstance(current_value, Parent): nested_values = current_value.model_dump(exclude_none=True) for nested_field, nested_value in nested_values.items(): full_path = f"{path}.{nested_field}" if path else nested_field diff --git a/layer/nrlf/core/tests/test_pydantic_errors.py b/layer/nrlf/core/tests/test_pydantic_errors.py index aae0da920..2978c7c15 100644 --- a/layer/nrlf/core/tests/test_pydantic_errors.py +++ b/layer/nrlf/core/tests/test_pydantic_errors.py @@ -242,37 +242,6 @@ def test_validate_content_invalid_content_stability_url(): } -def test_validate_content_missing_content_stability_coding(): - validator = DocumentReferenceValidator() - document_ref_data = load_document_reference_json("Y05868-736253002-Valid") - - # Remove the coding key from contentStability extension - del document_ref_data["content"][0]["extension"][0]["valueCodeableConcept"][ - "coding" - ] - - with pytest.raises(ParseError) as error: - validator.validate(document_ref_data) - - exc = error.value - assert len(exc.issues) == 1 - assert exc.issues[0].model_dump(exclude_none=True) == { - "severity": "error", - "code": "invalid", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_RESOURCE", - "display": "Invalid validation of resource", - } - ] - }, - "diagnostics": "Failed to parse DocumentReference resource (content[0].extension[0].valueCodeableConcept.coding: Field required. See ValueSet: https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability)", - "expression": ["content[0].extension[0].valueCodeableConcept.coding"], - } - - def test_validate_multiple_codings(): validator = DocumentReferenceValidator() document_ref_data = load_document_reference_json("Y05868-736253002-Valid") diff --git a/layer/nrlf/core/tests/test_validators.py b/layer/nrlf/core/tests/test_validators.py index 8c5c36e0f..fc4c824e4 100644 --- a/layer/nrlf/core/tests/test_validators.py +++ b/layer/nrlf/core/tests/test_validators.py @@ -331,62 +331,6 @@ def test_validate_document_reference_extra_fields_content(): } -def test_validate_identifiers_no_custodian_identifier(): - validator = DocumentReferenceValidator() - document_ref_data = load_document_reference_json("Y05868-736253002-Valid") - - del document_ref_data["custodian"]["identifier"] - - result = validator.validate(document_ref_data) - - assert result.is_valid is False - assert result.resource.id == "Y05868-99999-99999-999999" - assert len(result.issues) == 1 - assert result.issues[0].model_dump(exclude_none=True) == { - "severity": "error", - "code": "required", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_RESOURCE", - "display": "Invalid validation of resource", - } - ] - }, - "diagnostics": "Custodian must have an identifier", - "expression": ["custodian.identifier"], - } - - -def test_validate_identifiers_no_subject_identifier(): - validator = DocumentReferenceValidator() - document_ref_data = load_document_reference_json("Y05868-736253002-Valid") - - del document_ref_data["subject"]["identifier"] - - result = validator.validate(document_ref_data) - - assert result.is_valid is False - assert result.resource.id == "Y05868-99999-99999-999999" - assert len(result.issues) == 1 - assert result.issues[0].model_dump(exclude_none=True) == { - "severity": "error", - "code": "required", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_RESOURCE", - "display": "Invalid validation of resource", - } - ] - }, - "diagnostics": "Subject must have an identifier", - "expression": ["subject.identifier"], - } - - def test_validate_category_too_many_category(): validator = DocumentReferenceValidator() document_ref_data = load_document_reference_json("Y05868-736253002-Valid") @@ -931,33 +875,6 @@ def test_validate_relates_to_invalid_code(): } -def test_validate_relates_to_no_target_identifier(): - validator = DocumentReferenceValidator() - document_ref_data = load_document_reference_json("Y05868-736253002-Valid") - - document_ref_data["relatesTo"] = [{"code": "replaces", "target": {}}] - - result = validator.validate(document_ref_data) - - assert result.is_valid is False - assert len(result.issues) == 1 - assert result.issues[0].model_dump(exclude_none=True) == { - "severity": "error", - "code": "required", - "details": { - "coding": [ - { - "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", - "code": "INVALID_IDENTIFIER_VALUE", - "display": "Invalid identifier value", - } - ] - }, - "diagnostics": "relatesTo code 'replaces' must have a target identifier", - "expression": ["relatesTo[0].target.identifier.value"], - } - - def test_validate_ssp_content_with_asid(): validator = DocumentReferenceValidator() document_ref_data = load_document_reference_json( From 21677677419ae960b377baf3a5be04585d47727f Mon Sep 17 00:00:00 2001 From: "Axel Garcia K." Date: Tue, 29 Apr 2025 14:17:45 +0100 Subject: [PATCH 08/12] NRL-1285 Replace root in diagnostics for DocumentReference --- .../tests/test_create_document_reference.py | 8 ++++---- .../tests/test_upsert_document_reference.py | 4 ++-- layer/nrlf/core/errors.py | 4 ++-- layer/nrlf/core/tests/test_request.py | 8 ++++---- .../createDocumentReference-failure.feature | 16 ++++++++-------- .../updateDocumentReference-failure.feature | 8 ++++---- .../upsertDocumentReference-failure.feature | 8 ++++---- 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index 4fe10c800..ea9300ed9 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -421,8 +421,8 @@ def test_create_document_reference_empty_fields_in_body(): } ], }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: text, author, custodian.reference, custodian.identifier, category[0].coding[0].system, category[0].coding[0].code)", - "expression": ["root"], + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: text, author, custodian.reference, custodian.identifier, category[0].coding[0].system, category[0].coding[0].code)", + "expression": ["DocumentReference"], } ], } @@ -859,8 +859,8 @@ def test_create_document_reference_no_relatesto_target(): } ], }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: relatesTo[0].target)", - "expression": ["root"], + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: relatesTo[0].target)", + "expression": ["DocumentReference"], } ], } diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 3618e5624..e3a9e98fd 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -782,8 +782,8 @@ def test_upsert_document_reference_no_relatesto_target(): } ], }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: relatesTo[0].target)", - "expression": ["root"], + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: relatesTo[0].target)", + "expression": ["DocumentReference"], } ], } diff --git a/layer/nrlf/core/errors.py b/layer/nrlf/core/errors.py index f47df92bc..4280801b0 100644 --- a/layer/nrlf/core/errors.py +++ b/layer/nrlf/core/errors.py @@ -35,13 +35,13 @@ def append_value_set_url(loc_string: str) -> str: def diag_for_error(error: ErrorDetails) -> str: loc_string = format_error_location(error["loc"]) - msg = f"{loc_string or 'root'}: {error['msg']}" + msg = f"{loc_string or 'DocumentReference'}: {error['msg']}" msg += append_value_set_url(loc_string) return msg def expression_for_error(error: ErrorDetails) -> Optional[str]: - return format_error_location(error["loc"]) or "root" + return format_error_location(error["loc"]) or "DocumentReference" class OperationOutcomeError(Exception): diff --git a/layer/nrlf/core/tests/test_request.py b/layer/nrlf/core/tests/test_request.py index 726d1f520..22a98f207 100644 --- a/layer/nrlf/core/tests/test_request.py +++ b/layer/nrlf/core/tests/test_request.py @@ -245,8 +245,8 @@ def test_parse_body_invalid_docref_json(): } ], }, - "diagnostics": "Request body could not be parsed (root: Invalid JSON: control character (\\u0000-\\u001F) found while parsing a string at line 72 column 0)", - "expression": ["root"], + "diagnostics": "Request body could not be parsed (DocumentReference: Invalid JSON: control character (\\u0000-\\u001F) found while parsing a string at line 72 column 0)", + "expression": ["DocumentReference"], } ], } @@ -403,8 +403,8 @@ def test_parse_body_not_json(): } ] }, - "diagnostics": "Request body could not be parsed (root: Invalid JSON: expected value at line 1 column 1)", - "expression": ["root"], + "diagnostics": "Request body could not be parsed (DocumentReference: Invalid JSON: expected value at line 1 column 1)", + "expression": ["DocumentReference"], } ], } diff --git a/tests/features/producer/createDocumentReference-failure.feature b/tests/features/producer/createDocumentReference-failure.feature index 864e38bf2..d29116958 100644 --- a/tests/features/producer/createDocumentReference-failure.feature +++ b/tests/features/producer/createDocumentReference-failure.feature @@ -744,9 +744,9 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content)", + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: content)", "expression": [ - "root" + "DocumentReference" ] } """ @@ -802,9 +802,9 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content[0].attachment.contentType)", + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: content[0].attachment.contentType)", "expression": [ - "root" + "DocumentReference" ] } """ @@ -1008,9 +1008,9 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: context.practiceSetting.coding[0].display)", + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: context.practiceSetting.coding[0].display)", "expression": [ - "root" + "DocumentReference" ] } """ @@ -1040,7 +1040,7 @@ Feature: Producer - createDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: author)", - "expression": ["root"] + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: author)", + "expression": ["DocumentReference"] } """ diff --git a/tests/features/producer/updateDocumentReference-failure.feature b/tests/features/producer/updateDocumentReference-failure.feature index f9065ac67..527efe7b0 100644 --- a/tests/features/producer/updateDocumentReference-failure.feature +++ b/tests/features/producer/updateDocumentReference-failure.feature @@ -87,9 +87,9 @@ Feature: Producer - updateDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content)", + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: content)", "expression": [ - "root" + "DocumentReference" ] } """ @@ -158,9 +158,9 @@ Feature: Producer - updateDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content[0].attachment.contentType)", + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: content[0].attachment.contentType)", "expression": [ - "root" + "DocumentReference" ] } """ diff --git a/tests/features/producer/upsertDocumentReference-failure.feature b/tests/features/producer/upsertDocumentReference-failure.feature index c08598ccc..1e155b6a2 100644 --- a/tests/features/producer/upsertDocumentReference-failure.feature +++ b/tests/features/producer/upsertDocumentReference-failure.feature @@ -265,9 +265,9 @@ Feature: Producer - upsertDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content)", + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: content)", "expression": [ - "root" + "DocumentReference" ] } """ @@ -323,9 +323,9 @@ Feature: Producer - upsertDocumentReference - Failure Scenarios } ] }, - "diagnostics": "Request body could not be parsed (root: Value error, The following fields are empty: content[0].attachment.contentType)", + "diagnostics": "Request body could not be parsed (DocumentReference: Value error, The following fields are empty: content[0].attachment.contentType)", "expression": [ - "root" + "DocumentReference" ] } """ From c11522c7848a4551870557f7a41d7e09ec5a5ca0 Mon Sep 17 00:00:00 2001 From: eesa456 Date: Wed, 30 Apr 2025 00:45:09 +0100 Subject: [PATCH 09/12] NRL-1285 reduce complexity --- layer/nrlf/core/parent_model.py | 119 +++++++++++++++++++------------- 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/layer/nrlf/core/parent_model.py b/layer/nrlf/core/parent_model.py index fc1c2cb2b..e3138b3b2 100644 --- a/layer/nrlf/core/parent_model.py +++ b/layer/nrlf/core/parent_model.py @@ -88,8 +88,40 @@ def validate_empty_fields(cls, values): Iteratively check every field in the model for emptiness. If a field is empty, add it to the error list with its full location. """ - allowed_classes = [ + allowed_classes = cls.get_allowed_classes() + if cls.__name__ not in allowed_classes or not values: + return values + + stack = [(None, values)] + empty_fields = [] + + while stack: + path, current_value = stack.pop() + + if isinstance(current_value, dict): + cls.handle_dict(current_value, path, stack, empty_fields) + elif isinstance(current_value, list): + cls.handle_list(current_value, path, stack, empty_fields) + elif isinstance(current_value, Parent): + cls.handle_nested_model(current_value, path, stack) + else: + cls.handle_scalar(current_value, path, empty_fields) + + if empty_fields: + raise ValueError( + f"The following fields are empty: {', '.join(empty_fields)}" + ) + + return values + + @staticmethod + def get_allowed_classes(): + """ + Return the list of allowed classes for validation. + """ + return [ "DocumentReference", + "Meta", "Narrative", "Identifier", "NRLCodeableConcept", @@ -107,57 +139,50 @@ def validate_empty_fields(cls, values): "DocumentReferenceContext", "Period", ] - if cls.__name__ not in allowed_classes or not values: - return values - stack = [(None, values)] - empty_fields = [] - - while stack: - path, current_value = stack.pop() - - if isinstance(current_value, dict): - for key, value in current_value.items(): - full_path = f"{path}.{key}" if path else key - if ( - value is None - or value == "" - or (isinstance(value, list) and not value) - ): - empty_fields.append(full_path) - else: - stack.append((full_path, value)) - if not current_value: - empty_fields.append(path) - - elif isinstance(current_value, list): - for index, item in enumerate(current_value): - full_path = f"{path}[{index}]" if path else f"[{index}]" - if ( - item is None - or item == "" - or (isinstance(item, dict) and not item) - ): - empty_fields.append(full_path) - else: - stack.append((full_path, item)) - - elif isinstance(current_value, Parent): - nested_values = current_value.model_dump(exclude_none=True) - for nested_field, nested_value in nested_values.items(): - full_path = f"{path}.{nested_field}" if path else nested_field - stack.append((full_path, nested_value)) + @staticmethod + def handle_dict(current_value, path, stack, empty_fields): + """ + Handle validation for dictionary fields. + """ + for key, value in current_value.items(): + full_path = f"{path}.{key}" if path else key + if value is None or value == "" or (isinstance(value, list) and not value): + empty_fields.append(full_path) + else: + stack.append((full_path, value)) + if not current_value: + empty_fields.append(path) + @staticmethod + def handle_list(current_value, path, stack, empty_fields): + """ + Handle validation for list fields. + """ + for index, item in enumerate(current_value): + full_path = f"{path}[{index}]" if path else f"[{index}]" + if item is None or item == "" or (isinstance(item, dict) and not item): + empty_fields.append(full_path) else: - if current_value is None or current_value == "": - empty_fields.append(path) + stack.append((full_path, item)) - if empty_fields: - raise ValueError( - f"The following fields are empty: {', '.join(empty_fields)}" - ) + @staticmethod + def handle_nested_model(current_value, path, stack): + """ + Handle validation for nested Pydantic models. + """ + nested_values = current_value.model_dump(exclude_none=True) + for nested_field, nested_value in nested_values.items(): + full_path = f"{path}.{nested_field}" if path else nested_field + stack.append((full_path, nested_value)) - return values + @staticmethod + def handle_scalar(current_value, path, empty_fields): + """ + Handle validation for scalar fields. + """ + if current_value is None or current_value == "": + empty_fields.append(path) model_config = ConfigDict(regex_engine="python-re", extra="forbid") extension: Annotated[ From 8a520d44627b381a82e8eba49d47f2e6824d81d2 Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 1 May 2025 00:48:50 +0100 Subject: [PATCH 10/12] NRL-1285 only validate doc ref fields --- layer/nrlf/core/parent_model.py | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/layer/nrlf/core/parent_model.py b/layer/nrlf/core/parent_model.py index e3138b3b2..7f2f63bb3 100644 --- a/layer/nrlf/core/parent_model.py +++ b/layer/nrlf/core/parent_model.py @@ -88,8 +88,7 @@ def validate_empty_fields(cls, values): Iteratively check every field in the model for emptiness. If a field is empty, add it to the error list with its full location. """ - allowed_classes = cls.get_allowed_classes() - if cls.__name__ not in allowed_classes or not values: + if cls.__name__ != "DocumentReference" or not values: return values stack = [(None, values)] @@ -114,32 +113,6 @@ def validate_empty_fields(cls, values): return values - @staticmethod - def get_allowed_classes(): - """ - Return the list of allowed classes for validation. - """ - return [ - "DocumentReference", - "Meta", - "Narrative", - "Identifier", - "NRLCodeableConcept", - "NRLCoding", - "Reference", - "DocumentReferenceRelatesTo", - "CodeableConcept", - "Coding", - "DocumentReferenceContent", - "Attachment", - "NRLFormatCode", - "ContentStabilityExtension", - "ContentStabilityExtensionValueCodeableConcept", - "ContentStabilityExtensionCoding", - "DocumentReferenceContext", - "Period", - ] - @staticmethod def handle_dict(current_value, path, stack, empty_fields): """ From d0c73a6176bd3f2a0421c867704694310393a4a1 Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 1 May 2025 10:53:32 +0100 Subject: [PATCH 11/12] NRL-1285 change back to 400 for pydantic error --- .../tests/test_create_document_reference.py | 6 +++--- .../tests/test_upsert_document_reference.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index e1b41cce7..c3a96abf1 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -837,7 +837,7 @@ def test_create_document_reference_no_relatesto_target(): body = result.pop("body") assert result == { - "statusCode": "422", + "statusCode": "400", "headers": default_response_headers(), "isBase64Encoded": False, } @@ -853,8 +853,8 @@ def test_create_document_reference_no_relatesto_target(): "details": { "coding": [ { - "code": "UNPROCESSABLE_ENTITY", - "display": "Unprocessable Entity", + "code": "MESSAGE_NOT_WELL_FORMED", + "display": "Message not well formed", "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", } ], diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index ad33ac24f..43423a007 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -760,7 +760,7 @@ def test_upsert_document_reference_no_relatesto_target(): body = result.pop("body") assert result == { - "statusCode": "422", + "statusCode": "400", "headers": default_response_headers(), "isBase64Encoded": False, } @@ -776,8 +776,8 @@ def test_upsert_document_reference_no_relatesto_target(): "details": { "coding": [ { - "code": "UNPROCESSABLE_ENTITY", - "display": "Unprocessable Entity", + "code": "MESSAGE_NOT_WELL_FORMED", + "display": "Message not well formed", "system": "https://fhir.nhs.uk/ValueSet/Spine-ErrorOrWarningCode-1", } ], From c28069fd891a25cf2375c5a5d635cbbdad591e36 Mon Sep 17 00:00:00 2001 From: eesa456 Date: Thu, 1 May 2025 10:57:35 +0100 Subject: [PATCH 12/12] NRL-1285 fix tests --- .../tests/test_create_document_reference.py | 2 +- .../tests/test_upsert_document_reference.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/producer/createDocumentReference/tests/test_create_document_reference.py b/api/producer/createDocumentReference/tests/test_create_document_reference.py index c3a96abf1..73496262e 100644 --- a/api/producer/createDocumentReference/tests/test_create_document_reference.py +++ b/api/producer/createDocumentReference/tests/test_create_document_reference.py @@ -849,7 +849,7 @@ def test_create_document_reference_no_relatesto_target(): "issue": [ { "severity": "error", - "code": "business-rule", + "code": "invalid", "details": { "coding": [ { diff --git a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py index 43423a007..8ea04b281 100644 --- a/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py +++ b/api/producer/upsertDocumentReference/tests/test_upsert_document_reference.py @@ -772,7 +772,7 @@ def test_upsert_document_reference_no_relatesto_target(): "issue": [ { "severity": "error", - "code": "business-rule", + "code": "invalid", "details": { "coding": [ {