NRL-1554 Allow arbitrary extensions in content, change diagnostic messages

axelkrastek1-nhs · axelkrastek1-nhs · commit bf58542cc161 · 2025-08-18T09:21:04.000+01:00
diff --git a/api/consumer/swagger.yaml b/api/consumer/swagger.yaml
@@ -813,9 +813,9 @@ components:
             oneOf:
               - $ref: "#/components/schemas/ContentStabilityExtension"
               - $ref: "#/components/schemas/RetrievalMechanismExtension"
-            description: Additional extension for content stability.
+              - $ref: "#/components/schemas/Extension"
+            description: Additional extensions which include Content Stability and Retrieval Mechanism.
           minItems: 1
-          maxItems: 2
       required:
         - attachment
         - format
diff --git a/api/producer/swagger.yaml b/api/producer/swagger.yaml
@@ -1418,9 +1418,9 @@ components:
             oneOf:
               - $ref: "#/components/schemas/ContentStabilityExtension"
               - $ref: "#/components/schemas/RetrievalMechanismExtension"
-            description: Additional extension for content stability.
+              - $ref: "#/components/schemas/Extension"
+            description: Additional extensions which include Content Stability and Retrieval Mechanism.
           minItems: 1
-          maxItems: 2
       required:
         - attachment
         - format
diff --git a/layer/nrlf/consumer/fhir/r4/model.py b/layer/nrlf/consumer/fhir/r4/model.py
@@ -1,6 +1,6 @@
 # generated by datamodel-codegen:
 #   filename:  swagger.yaml
-#   timestamp: 2025-08-07T11:40:24+00:00
+#   timestamp: 2025-08-14T08:25:07+00:00
 
 from __future__ import annotations
 
@@ -652,8 +652,8 @@ class DocumentReferenceContent(Parent):
         ),
     ]
     extension: Annotated[
-        List[Union[ContentStabilityExtension, RetrievalMechanismExtension]],
-        Field(max_length=2, min_length=1),
+        List[Union[ContentStabilityExtension, RetrievalMechanismExtension, Extension]],
+        Field(min_length=1),
     ]
 
 
diff --git a/layer/nrlf/core/errors.py b/layer/nrlf/core/errors.py
@@ -3,53 +3,41 @@
 from pydantic import ValidationError
 from pydantic_core import ErrorDetails
 
-from nrlf.core.constants import (
-    CONTENT_FORMAT_CODE_URL,
-    CONTENT_RETRIEVAL_SYSTEM_URL,
-    CONTENT_STABILITY_SYSTEM_URL,
-)
 from nrlf.core.response import Response
 from nrlf.core.types import CodeableConcept
 from nrlf.producer.fhir.r4 import model as producer_model
 from nrlf.producer.fhir.r4.model import OperationOutcome, OperationOutcomeIssue
 
 
 def format_error_location(loc: List) -> str:
-    # List of extension class names to exclude from error paths
-    exclude_classes = {"ContentStabilityExtension", "RetrievalMechanismExtension"}
-    filtered_loc = [each for each in loc if each not in exclude_classes]
-
     formatted_loc = ""
-    for each in filtered_loc:
+    for each in loc:
         if isinstance(each, int):
             formatted_loc = f"{formatted_loc}[{each}]"
         else:
             formatted_loc = f"{formatted_loc}.{each}" if formatted_loc else str(each)
     return formatted_loc
 
 
-def append_value_set_url(loc_string: str) -> str:
-    if loc_string.endswith(("url", "system")):
-        return ""
-
-    if "content" in loc_string:
-        if "extension" in loc_string:
-            return f". See ValueSets: {CONTENT_STABILITY_SYSTEM_URL} & {CONTENT_RETRIEVAL_SYSTEM_URL}"
-        if "format" in loc_string:
-            return f". See ValueSet: {CONTENT_FORMAT_CODE_URL}"
-
-    return ""
-
-
-def diag_for_error(error: ErrorDetails) -> str:
+def diag_for_error(error: ErrorDetails, value_set: str, root_location: tuple) -> str:
     loc_string = format_error_location(error["loc"])
+    if root_location:
+        loc_string = format_error_location(root_location) + "." + loc_string
+
     msg = f"{loc_string or 'DocumentReference'}: {error['msg']}"
-    msg += append_value_set_url(loc_string)
+    msg += f", see: {value_set}" if value_set else ""
     return msg
 
 
-def expression_for_error(error: ErrorDetails) -> Optional[str]:
-    return format_error_location(error["loc"]) or "DocumentReference"
+def expression_for_error(error: ErrorDetails, root_location: tuple) -> Optional[str]:
+    loc_string = format_error_location(error["loc"]) or "DocumentReference"
+    if root_location and error["loc"]:
+        loc_string = (
+            format_error_location(root_location)
+            + "."
+            + format_error_location(error["loc"])
+        )
+    return loc_string
 
 
 class OperationOutcomeError(Exception):
@@ -99,15 +87,20 @@ def __init__(self, issues: List[OperationOutcomeIssue]):
 
     @classmethod
     def from_validation_error(
-        cls, exc: ValidationError, details: CodeableConcept, msg: str = ""
+        cls,
+        exc: ValidationError,
+        details: CodeableConcept,
+        msg: str = "",
+        value_set: str = "",
+        root_location: tuple = None,
     ):
         issues = [
             producer_model.OperationOutcomeIssue(
                 severity="error",
                 code="invalid",
                 details=details,  # type: ignore
-                diagnostics=f"{msg} ({diag_for_error(error)})",
-                expression=[expression_for_error(error)],  # type: ignore
+                diagnostics=f"{msg} ({diag_for_error(error, value_set, root_location)})",
+                expression=[expression_for_error(error, root_location)],  # type: ignore
             )
             for error in exc.errors()
         ]
diff --git a/layer/nrlf/core/tests/test_pydantic_errors.py b/layer/nrlf/core/tests/test_pydantic_errors.py
@@ -85,47 +85,11 @@ def test_validate_content_missing_format():
                 }
             ]
         },
-        "diagnostics": "Failed to parse DocumentReference resource (content[0].format: Field required. See ValueSet: https://fhir.nhs.uk/England/CodeSystem/England-NRLFormatCode)",
+        "diagnostics": "Failed to parse DocumentReference resource (content[0].format: Field required)",
         "expression": ["content[0].format"],
     }
 
 
-def test_validate_content_multiple_content_stability_extensions():
-    validator = DocumentReferenceValidator()
-    document_ref_data = load_document_reference_json("Y05868-736253002-Valid")
-
-    # Add a second duplicate contentStability extension
-    document_ref_data["content"][0]["extension"].append(
-        document_ref_data["content"][0]["extension"][0]
-    )
-
-    # Add a third duplicate contentStability extension
-    document_ref_data["content"][0]["extension"].append(
-        document_ref_data["content"][0]["extension"][0]
-    )
-
-    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/CodeSystem/Spine-ErrorOrWarningCode",
-                    "code": "BAD_REQUEST",
-                    "display": "Bad request",
-                }
-            ]
-        },
-        "diagnostics": "Failed to parse DocumentReference resource (content[0].extension: List should have at most 2 items after validation, not 3. See ValueSets: https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability & https://fhir.nhs.uk/England/CodeSystem/England-RetrievalMechanism)",
-        "expression": ["content[0].extension"],
-    }
-
-
 def test_validate_content_invalid_content_stability_code():
     validator = DocumentReferenceValidator()
     document_ref_data = load_document_reference_json("Y05868-736253002-Valid")
@@ -150,7 +114,7 @@ def test_validate_content_invalid_content_stability_code():
                 }
             ]
         },
-        "diagnostics": "Failed to parse DocumentReference resource (content[0].extension[0].valueCodeableConcept.coding[0].code: Input should be 'static' or 'dynamic'. See ValueSets: https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability & https://fhir.nhs.uk/England/CodeSystem/England-RetrievalMechanism)",
+        "diagnostics": "Invalid content stability extension (content[0].extension[0].valueCodeableConcept.coding[0].code: Input should be 'static' or 'dynamic', see: https://fhir.nhs.uk/England/ValueSet/England-NRLContentStability)",
         "expression": ["content[0].extension[0].valueCodeableConcept.coding[0].code"],
     }
 
@@ -179,7 +143,7 @@ def test_validate_content_invalid_content_stability_display():
                 }
             ]
         },
-        "diagnostics": "Failed to parse DocumentReference resource (content[0].extension[0].valueCodeableConcept.coding[0].display: Input should be 'Static' or 'Dynamic'. See ValueSets: https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability & https://fhir.nhs.uk/England/CodeSystem/England-RetrievalMechanism)",
+        "diagnostics": "Invalid content stability extension (content[0].extension[0].valueCodeableConcept.coding[0].display: Input should be 'Static' or 'Dynamic', see: https://fhir.nhs.uk/England/ValueSet/England-NRLContentStability)",
         "expression": [
             "content[0].extension[0].valueCodeableConcept.coding[0].display"
         ],
@@ -210,7 +174,7 @@ def test_validate_content_invalid_content_stability_system():
                 }
             ]
         },
-        "diagnostics": "Failed to parse DocumentReference resource (content[0].extension[0].valueCodeableConcept.coding[0].system: Input should be 'https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability')",
+        "diagnostics": "Invalid content stability extension (content[0].extension[0].valueCodeableConcept.coding[0].system: Input should be 'https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability', see: https://fhir.nhs.uk/England/ValueSet/England-NRLContentStability)",
         "expression": ["content[0].extension[0].valueCodeableConcept.coding[0].system"],
     }
 
@@ -238,7 +202,7 @@ def test_validate_content_invalid_content_stability_url():
                 }
             ]
         },
-        "diagnostics": "Failed to parse DocumentReference resource (content[0].extension[0].url: Input should be 'https://fhir.nhs.uk/England/StructureDefinition/Extension-England-ContentStability')",
+        "diagnostics": "Invalid content stability extension (content[0].extension[0].url: Input should be 'https://fhir.nhs.uk/England/StructureDefinition/Extension-England-ContentStability', see: https://fhir.nhs.uk/England/ValueSet/England-NRLContentStability)",
         "expression": ["content[0].extension[0].url"],
     }
 
diff --git a/layer/nrlf/core/tests/test_validators.py b/layer/nrlf/core/tests/test_validators.py
@@ -1402,7 +1402,7 @@ def test_validate_content_extension_missing_content_stability():
                 }
             ]
         },
-        "diagnostics": "Invalid content extension: Extension must have one content stability extension see value set ('https://fhir.nhs.uk/England/CodeSystem/England-NRLContentStability')",
+        "diagnostics": "Invalid content extension: Extension must have one content stability extension, see: ('https://fhir.nhs.uk/England/ValueSet/England-NRLContentStability')",
         "expression": ["content[0].extension"],
     }
 
@@ -1559,3 +1559,74 @@ def test_validate_nrl_format_code_display_mismatch(
         "diagnostics": f"Invalid display for format code '{format_code}'. Expected '{expected_display}'",
         "expression": ["content[0].format.display"],
     }
+
+
+def test_validate_content_multiple_content_stability_extensions():
+    validator = DocumentReferenceValidator()
+    document_ref_data = load_document_reference_json("Y05868-736253002-Valid")
+
+    # Add a second duplicate contentStability extension
+    document_ref_data["content"][0]["extension"].append(
+        document_ref_data["content"][0]["extension"][0]
+    )
+
+    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": "business-rule",
+        "details": {
+            "coding": [
+                {
+                    "system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode",
+                    "code": "UNPROCESSABLE_ENTITY",
+                    "display": "Unprocessable Entity",
+                }
+            ]
+        },
+        "diagnostics": "Invalid content extension: Extension must have one content stability extension, see: ('https://fhir.nhs.uk/England/ValueSet/England-NRLContentStability')",
+        "expression": ["content[0].extension"],
+    }
+
+
+def test_validate_content_multiple_content_retrieval_extensions():
+    validator = DocumentReferenceValidator()
+    document_ref_data = load_document_reference_json("Y05868-736253002-Valid")
+
+    # Add 2 content retrieval extensions
+    content_retrieval_extension = {
+        "url": "https://fhir.nhs.uk/England/StructureDefinition/Extension-England-RetrievalMechanism",
+        "valueCodeableConcept": {
+            "coding": [
+                {
+                    "system": "https://fhir.nhs.uk/England/CodeSystem/England-RetrievalMechanism",
+                    "code": "Direct",
+                    "display": "Direct",
+                }
+            ]
+        },
+    }
+    document_ref_data["content"][0]["extension"].append(content_retrieval_extension)
+    document_ref_data["content"][0]["extension"].append(content_retrieval_extension)
+
+    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": "business-rule",
+        "details": {
+            "coding": [
+                {
+                    "system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode",
+                    "code": "UNPROCESSABLE_ENTITY",
+                    "display": "Unprocessable Entity",
+                }
+            ]
+        },
+        "diagnostics": "Invalid content retrieval extension: Extension must have one content retrieval extension, see: ('https://fhir.nhs.uk/England/ValueSet/England-RetrievalMechanism')",
+        "expression": ["content[0].extension"],
+    }
diff --git a/layer/nrlf/core/validators.py b/layer/nrlf/core/validators.py
diff --git a/layer/nrlf/producer/fhir/r4/model.py b/layer/nrlf/producer/fhir/r4/model.py
diff --git a/layer/nrlf/producer/fhir/r4/strict_model.py b/layer/nrlf/producer/fhir/r4/strict_model.py

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`# generated by datamodel-codegen:`
`2`	`2`	`# filename: swagger.yaml`
`3`		`-# timestamp: 2025-08-07T11:40:24+00:00`
	`3`	`+# timestamp: 2025-08-14T08:25:07+00:00`
`4`	`4`
`5`	`5`	`from __future__ import annotations`
`6`	`6`
`@@ -652,8 +652,8 @@ class DocumentReferenceContent(Parent):`
`652`	`652`	`),`
`653`	`653`	`]`
`654`	`654`	`extension: Annotated[`
`655`		`- List[Union[ContentStabilityExtension, RetrievalMechanismExtension]],`
`656`		`- Field(max_length=2, min_length=1),`
	`655`	`+ List[Union[ContentStabilityExtension, RetrievalMechanismExtension, Extension]],`
	`656`	`+ Field(min_length=1),`
`657`	`657`	`]`
`658`	`658`
`659`	`659`