Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,10 @@ def handler(
return error_response

can_ignore_delete_fail = (
PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value
in metadata.nrl_permissions_policy.access_controls
if metadata.nrl_permissions_policy
else PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
)

if ids_to_delete := _get_document_ids_to_supersede(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1435,7 +1435,7 @@ def test_create_document_reference_supersede_deletes_old_pointers_replace(
@mock_aws
@mock_repository
@freeze_uuid("00000000-0000-0000-0000-000000000001")
def test_create_document_reference_supersede_succeeds_with_toggle(
def test_supersede_non_existent_pointer_succeeds_with_v1_ignore_delete_fail(
repository: DocumentPointerRepository,
):
doc_ref = load_document_reference("Y05868-736253002-Valid")
Expand Down Expand Up @@ -1493,7 +1493,7 @@ def test_create_document_reference_supersede_succeeds_with_toggle(

@mock_aws
@mock_repository
def test_create_document_reference_supersede_fails_without_toggle(
def test_supersede_non_existent_pointer_fails_without_v1_ignore_delete_fail(
repository: DocumentPointerRepository,
):
doc_ref = load_document_reference("Y05868-736253002-Valid")
Expand Down Expand Up @@ -1544,6 +1544,143 @@ def test_create_document_reference_supersede_fails_without_toggle(
}


@mock_aws
@mock_repository
@freeze_uuid("00000000-0000-0000-0000-000000000001")
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
def test_supersede_non_existent_pointer_succeeds_with_v2_access_control(
get_pointer_permissions_mock, repository: DocumentPointerRepository
):
doc_ref = load_document_reference("Y05868-736253002-Valid")

# Add reference to a non-existing pointer
doc_ref.relatesTo = [
DocumentReferenceRelatesTo(
code="replaces",
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
)
]

v2_headers = create_headers(
additional_headers={
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
}
)
v2_headers.pop(CLIENT_RP_DETAILS)

get_pointer_permissions_mock.return_value = {
"access_controls": [AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value],
"types": ["http://snomed.info/sct|736253002"],
}

event = create_test_api_gateway_event(
headers=v2_headers,
body=doc_ref.model_dump_json(exclude_none=True),
)

result = handler(event, create_mock_context())
body = result.pop("body")

assert result == {
"statusCode": "201",
"headers": {
"Location": "/DocumentReference/Y05868-00000000-0000-0000-0000-000000000001",
**default_response_headers(),
},
"isBase64Encoded": False,
}

parsed_body = json.loads(body)

assert parsed_body == {
"resourceType": "OperationOutcome",
"issue": [
{
"severity": "information",
"code": "informational",
"details": {
"coding": [
{
"code": "RESOURCE_SUPERSEDED",
"display": "Resource created and resource(s) deleted",
"system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode",
}
]
},
"diagnostics": "The document has been superseded by a new version",
}
],
}


@mock_aws
@mock_repository
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
def test_supersede_fails_without_v2_access_control(
get_pointer_permissions_mock, repository: DocumentPointerRepository
):
doc_ref = load_document_reference("Y05868-736253002-Valid")

# Add reference to a non-existing pointer
doc_ref.relatesTo = [
DocumentReferenceRelatesTo(
code="replaces",
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
)
]

v2_headers = create_headers(
additional_headers={
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
}
)
v2_headers.pop(CLIENT_RP_DETAILS)

get_pointer_permissions_mock.return_value = {
"access_controls": [],
"types": ["http://snomed.info/sct|736253002"],
}

event = create_test_api_gateway_event(
headers=v2_headers,
body=doc_ref.model_dump_json(exclude_none=True),
)

result = handler(event, create_mock_context())
body = result.pop("body")

assert result == {
"statusCode": "422",
"headers": default_response_headers(),
"isBase64Encoded": False,
}

parsed_body = json.loads(body)

assert parsed_body == {
"resourceType": "OperationOutcome",
"issue": [
{
"severity": "error",
"code": "business-rule",
"details": {
"coding": [
{
"code": "UNPROCESSABLE_ENTITY",
"display": "Unprocessable Entity",
"system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode",
}
]
},
"diagnostics": "The relatesTo target document does not exist",
"expression": ["relatesTo[0].target.identifier.value"],
}
],
}


@mock_aws
@mock_repository
@freeze_uuid("00000000-0000-0000-0000-000000000001")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,7 @@ def test_upsert_document_reference_supersede_deletes_old_pointers_replace(

@mock_aws
@mock_repository
def test_upsert_document_reference_supersede_succeeds_with_toggle(
def test_supersede_non_existent_pointer_succeeds_with_v1_ignore_delete_fail(
repository: DocumentPointerRepository,
):
doc_ref = load_document_reference("Y05868-736253002-Valid")
Expand Down Expand Up @@ -1463,13 +1463,10 @@ def test_upsert_document_reference_supersede_succeeds_with_toggle(
],
}

non_existent_pointer = repository.get_by_id("Y05868-99999-99999-000000")
assert non_existent_pointer is None


@mock_aws
@mock_repository
def test_upsert_document_reference_supersede_fails_without_toggle(
def test_supersede_non_existent_pointer_fails_without_v1_ignore_delete_fail(
repository: DocumentPointerRepository,
):
doc_ref = load_document_reference("Y05868-736253002-Valid")
Expand Down Expand Up @@ -1520,6 +1517,142 @@ def test_upsert_document_reference_supersede_fails_without_toggle(
}


@mock_aws
@mock_repository
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
def test_supersede_non_existent_pointer_succeeds_with_v2_access_control(
get_pointer_permissions_mock, repository: DocumentPointerRepository
):
doc_ref = load_document_reference("Y05868-736253002-Valid")

# Add reference to a non-existing pointer
doc_ref.relatesTo = [
DocumentReferenceRelatesTo(
code="replaces",
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
)
]

v2_headers = create_headers(
additional_headers={
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
}
)
v2_headers.pop(CLIENT_RP_DETAILS)

get_pointer_permissions_mock.return_value = {
"access_controls": [AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value],
"types": ["http://snomed.info/sct|736253002"],
}

event = create_test_api_gateway_event(
headers=v2_headers,
body=doc_ref.model_dump_json(exclude_none=True),
)

result = handler(event, create_mock_context())
body = result.pop("body")

assert result == {
"statusCode": "201",
"headers": {
"Location": "/DocumentReference/Y05868-99999-99999-999999",
**default_response_headers(),
},
"isBase64Encoded": False,
}

parsed_body = json.loads(body)

assert parsed_body == {
"resourceType": "OperationOutcome",
"issue": [
{
"severity": "information",
"code": "informational",
"details": {
"coding": [
{
"code": "RESOURCE_SUPERSEDED",
"display": "Resource created and resource(s) deleted",
"system": "https://fhir.nhs.uk/ValueSet/NRL-ResponseCode",
}
]
},
"diagnostics": "The document has been superseded by a new version",
}
],
}


@mock_aws
@mock_repository
@patch("nrlf.core.decorators.get_pointer_permissions_v2")
def test_supersede_fails_without_v2_access_control(
get_pointer_permissions_mock, repository: DocumentPointerRepository
):
doc_ref = load_document_reference("Y05868-736253002-Valid")

# Add reference to a non-existing pointer
doc_ref.relatesTo = [
DocumentReferenceRelatesTo(
code="replaces",
target=Reference(identifier=Identifier(value="Y05868-99999-99999-000000")),
)
]

v2_headers = create_headers(
additional_headers={
V2Headers.NHSD_END_USER_ORGANISATION_ODS: "Y05868",
V2Headers.NHSD_NRL_APP_ID: "Y05868-TestApp-12345678",
}
)
v2_headers.pop(CLIENT_RP_DETAILS)

get_pointer_permissions_mock.return_value = {
"access_controls": [],
"types": ["http://snomed.info/sct|736253002"],
}

event = create_test_api_gateway_event(
headers=v2_headers,
body=doc_ref.model_dump_json(exclude_none=True),
)

result = handler(event, create_mock_context())
body = result.pop("body")

assert result == {
"statusCode": "422",
"headers": default_response_headers(),
"isBase64Encoded": False,
}

parsed_body = json.loads(body)

assert parsed_body == {
"resourceType": "OperationOutcome",
"issue": [
{
"severity": "error",
"code": "business-rule",
"details": {
"coding": [
{
"code": "UNPROCESSABLE_ENTITY",
"display": "Unprocessable Entity",
"system": "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode",
}
]
},
"diagnostics": "The relatesTo target document does not exist",
"expression": ["relatesTo[0].target.identifier.value"],
}
],
}


@mock_aws
@mock_repository
def test_upsert_document_reference_create_relatesto_not_replaces(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ def handler(
return error_response

can_ignore_delete_fail = (
PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value
in metadata.nrl_permissions_policy.access_controls
if metadata.nrl_permissions_policy
else PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
)

if ids_to_delete := _get_document_ids_to_supersede(
Expand Down
1 change: 1 addition & 0 deletions layer/nrlf/core/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class AccessControls(Enum):
ALLOW_PRODUCE_FOR_ANY_AUTHOR = "allow_produce_for_any_author"
ALLOW_PRODUCE_FOR_ANY_CUSTODIAN = "allow_produce_for_any_custodian"
ALLOW_OVERRIDE_CREATION_DATETIME = "allow_override_creation_datetime"
ALLOW_SUPERSEDE_WITH_DELETE_FAILURE = "allow_supersede_with_delete_failure"

@staticmethod
def list():
Expand Down
8 changes: 8 additions & 0 deletions layer/nrlf/core/dynamodb/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,14 @@ def delete_by_id(self, id_: str, can_ignore_delete_fail: bool = False):
stacklevel=5,
error=str(exc),
)
# Is this what it should be doing, do we want to change the behaviour here?
# else:
# raise OperationOutcomeError(
# status_code="500",
# severity="error",
# code="exception",
# details=SpineErrorConcept.from_code("INTERNAL_SERVER_ERROR"),
# ) from exc

def _query(self, **kwargs) -> Iterator[DocumentPointer]:
"""
Expand Down
1 change: 1 addition & 0 deletions scripts/get_s3_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def add_feature_test_files(local_path):
[
AccessControls.ALLOW_ALL_TYPES.value,
AccessControls.ALLOW_OVERRIDE_CREATION_DATETIME.value,
AccessControls.ALLOW_SUPERSEDE_WITH_DELETE_FAILURE.value,
],
),
],
Expand Down
Loading
Loading