Merge pull request #251 from NHSDigital/release/2024-07-08

Release/2024-07-08

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 2024-07-08
+- [PI-451] Improves ETL logging, may fix [PI-450] by allowing ldif deletions by replacing with empty value
+- [PI-437] Sonarcloud improvements
+
 ## 2024-07-05
 - [PI-435] REDLINE Repo changes
 - [PI-360] Deployment of changelog

VERSION

@@ -1 +1 @@
-2024.07.05
+2024.07.08

changelog/2024-07-08.md

@@ -0,0 +1,2 @@
+- [PI-451] Improves ETL logging, may fix [PI-450] by allowing ldif deletions by replacing with empty value
+- [PI-437] Sonarcloud improvements

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "connecting-party-manager"
-version = "2024.07.05"
+version = "2024.07.08"
 description = "Repository for the Connecting Party Manager API and related services"
 authors = ["NHS England"]
 license = "LICENSE.md"

src/api/tests/feature_tests/steps/tests/test_endpoint_lambda_mapping.py

@@ -46,11 +46,11 @@ def test_parse_path_create_device():
 
         endpoint_lambda_mapping = get_endpoint_lambda_mapping()
 
-    parse_api_path(
+    assert parse_api_path(
         method="POST",
-        path="Organization",
+        path="Device",
         endpoint_lambda_mapping=endpoint_lambda_mapping,
-    ) == {}, {}, api.createDevice.index.handler
+    ) == ({}, {}, api.createDevice.index)
 
 
 def test_parse_path_read_device():
@@ -59,11 +59,11 @@ def test_parse_path_read_device():
 
         endpoint_lambda_mapping = get_endpoint_lambda_mapping()
 
-    parse_api_path(
+    assert parse_api_path(
         method="GET",
-        path="Organization/123",
+        path="Device/123",
         endpoint_lambda_mapping=endpoint_lambda_mapping,
-    ) == {"id": "123"}, {}, api.readDevice.index.handler
+    ) == ({"id": "123"}, {}, api.readDevice.index)
 
 
 def test_parse_path_create_product_team():
@@ -72,11 +72,11 @@ def test_parse_path_create_product_team():
 
         endpoint_lambda_mapping = get_endpoint_lambda_mapping()
 
-    parse_api_path(
+    assert parse_api_path(
         method="POST",
         path="Organization",
         endpoint_lambda_mapping=endpoint_lambda_mapping,
-    ) == {}, {}, api.createProductTeam.index.handler
+    ) == ({}, {}, api.createProductTeam.index)
 
 
 def test_parse_path_read_product_team():
@@ -85,11 +85,11 @@ def test_parse_path_read_product_team():
 
         endpoint_lambda_mapping = get_endpoint_lambda_mapping()
 
-    parse_api_path(
+    assert parse_api_path(
         method="GET",
         path="Organization/123",
         endpoint_lambda_mapping=endpoint_lambda_mapping,
-    ) == {"id": "123"}, {}, api.readProductTeam.index.handler
+    ) == ({"id": "123"}, {}, api.readProductTeam.index)
 
 
 def test_parse_path_error():

src/etl/sds/tests/changelog/changelog_components/modify/accredited_system/replace/nhs_product_name.WithEmpty.ldif

@@ -0,0 +1 @@
+replace: nhsProductName

src/etl/sds/tests/changelog/scenarios/accredited_system/replacing_optional_field_with_empty_field_sets_field_to_null/__init__.py

@@ -0,0 +1,20 @@
+from etl.sds.tests.changelog.utils import (
+    ADD_ACCREDITED_SYSTEM,
+    _Scenario,
+    create_modify_ldif,
+)
+
+_ADD_NHS_AS_CLIENT = create_modify_ldif(
+    "add/nhs_as_client.ldif", device_type="accredited_system"
+)
+_REPLACE_NHS_PRODUCT_NAME = create_modify_ldif(
+    "replace/nhs_product_name.WithEmpty.ldif", device_type="accredited_system"
+)
+SCENARIO = _Scenario(
+    file_path=__file__,
+    extract_input=[
+        ADD_ACCREDITED_SYSTEM,
+        _ADD_NHS_AS_CLIENT,
+        _REPLACE_NHS_PRODUCT_NAME,
+    ],
+)

src/etl/sds/tests/changelog/scenarios/accredited_system/replacing_optional_field_with_empty_field_sets_field_to_null/extract_output.json

@@ -0,0 +1,35 @@
+[
+  {
+    "change_type": "add",
+    "description": null,
+    "nhs_approver_urp": "uniqueIdentifier=102583034545,uniqueIdentifier=352307522545,uid=432776896545,ou=People,o=nhs",
+    "nhs_as_acf": null,
+    "nhs_as_category_bag": null,
+    "nhs_as_client": ["K83015"],
+    "nhs_as_svc_ia": ["urn:nhs:names:services:pdsquery:QUQI_IN010000UK14"],
+    "nhs_date_approved": "20091016133823",
+    "nhs_date_requested": "20091016133757",
+    "nhs_id_code": "K83015",
+    "nhs_mhs_manufacturer_org": null,
+    "nhs_mhs_party_key": "K83015-806782",
+    "nhs_product_key": "6216",
+    "nhs_product_name": "TPP SystmOne",
+    "nhs_product_version": null,
+    "nhs_requestor_urp": "uniqueIdentifier=203171972540,uniqueIdentifier=352307522545,uid=432776896545,ou=People,o=nhs",
+    "nhs_temp_uid": "10312",
+    "object_class": "nhsAS",
+    "unique_identifier": "000173655515"
+  },
+  {
+    "object_class": "modify",
+    "change_type": "modify",
+    "modifications": [["add", "nhsasclient", ["AAA"]]],
+    "unique_identifier": "000173655515"
+  },
+  {
+    "object_class": "modify",
+    "change_type": "modify",
+    "modifications": [["replace", "nhsproductname", []]],
+    "unique_identifier": "000173655515"
+  }
+]

src/etl/sds/tests/changelog/scenarios/accredited_system/replacing_optional_field_with_empty_field_sets_field_to_null/load_output.json

@@ -0,0 +1,96 @@
+[
+  {
+    "keys": {
+      "K83015:000173655515": {
+        "key": "K83015:000173655515",
+        "type": "accredited_system_id"
+      }
+    },
+    "name": "TPP SystmOne",
+    "ods_code": "K83015",
+    "product_team_id": "12345678-1234-5678-1234-567812345678",
+    "questionnaire_responses": {
+      "spine_device/1": [
+        {
+          "responses": [
+            { "object_class": ["nhsAS"] },
+            { "unique_identifier": ["000173655515"] },
+            {
+              "nhs_approver_urp": [
+                "uniqueIdentifier=102583034545,uniqueIdentifier=352307522545,uid=432776896545,ou=People,o=nhs"
+              ]
+            },
+
+            { "nhs_date_approved": ["20091016133823"] },
+            {
+              "nhs_requestor_urp": [
+                "uniqueIdentifier=203171972540,uniqueIdentifier=352307522545,uid=432776896545,ou=People,o=nhs"
+              ]
+            },
+
+            { "nhs_date_requested": ["20091016133757"] },
+            { "nhs_id_code": ["K83015"] },
+            { "nhs_mhs_party_key": ["K83015-806782"] },
+            { "nhs_product_key": ["6216"] },
+            { "nhs_as_client": ["K83015"] },
+            {
+              "nhs_as_svc_ia": [
+                "urn:nhs:names:services:pdsquery:QUQI_IN010000UK14"
+              ]
+            },
+            { "nhs_temp_uid": ["10312"] }
+          ]
+        }
+      ]
+    },
+    "status": "active",
+    "type": "product"
+  },
+  {
+    "keys": {
+      "AAA:000173655515": {
+        "key": "AAA:000173655515",
+        "type": "accredited_system_id"
+      }
+    },
+    "name": "TPP SystmOne",
+    "ods_code": "AAA",
+    "product_team_id": "12345678-1234-5678-1234-567812345678",
+    "questionnaire_responses": {
+      "spine_device/1": [
+        {
+          "responses": [
+            { "object_class": ["nhsAS"] },
+            { "unique_identifier": ["000173655515"] },
+            {
+              "nhs_approver_urp": [
+                "uniqueIdentifier=102583034545,uniqueIdentifier=352307522545,uid=432776896545,ou=People,o=nhs"
+              ]
+            },
+
+            { "nhs_date_approved": ["20091016133823"] },
+            {
+              "nhs_requestor_urp": [
+                "uniqueIdentifier=203171972540,uniqueIdentifier=352307522545,uid=432776896545,ou=People,o=nhs"
+              ]
+            },
+
+            { "nhs_date_requested": ["20091016133757"] },
+            { "nhs_id_code": ["K83015"] },
+            { "nhs_mhs_party_key": ["K83015-806782"] },
+            { "nhs_product_key": ["6216"] },
+            { "nhs_as_client": ["AAA"] },
+            {
+              "nhs_as_svc_ia": [
+                "urn:nhs:names:services:pdsquery:QUQI_IN010000UK14"
+              ]
+            },
+            { "nhs_temp_uid": ["10312"] }
+          ]
+        }
+      ]
+    },
+    "status": "active",
+    "type": "product"
+  }
+]

src/etl/sds/tests/test_sds_etl_components.py

@@ -78,26 +78,19 @@ def repository():
 
 
 def execute_state_machine(
-    state_machine_input: StateMachineInput, client
+    state_machine_input: StateMachineInput, step_functions_client
 ) -> StartSyncExecutionOutputTypeDef:
-    config = Config(
-        retries={
-            "max_attempts": 10,  # Increase the number of max attempts
-            "mode": "standard",  # Standard retry mode
-        }
-    )
-    client = boto3.client("stepfunctions", config=config)
     state_machine_arn = read_terraform_output("sds_etl.value.state_machine_arn")
     name = state_machine_input.name
-    execution_response = client.start_execution(
+    execution_response = step_functions_client.start_execution(
         stateMachineArn=state_machine_arn,
         name=name,
         input=json.dumps(state_machine_input.dict()),
     )
 
     status = "RUNNING"
     while status == "RUNNING":
-        response = client.describe_execution(
+        response = step_functions_client.describe_execution(
             executionArn=execution_response["executionArn"]
         )
         status = response["status"]
@@ -140,29 +133,15 @@ def put_object(s3_client, key: WorkerKey, body: bytes) -> str:
 
 
 @pytest.mark.integration
-@pytest.mark.parametrize(
-    "worker_data",
-    [
-        {
-            WorkerKey.EXTRACT: "",
-            WorkerKey.TRANSFORM: pkl_dumps_lz4(deque()),
-            WorkerKey.LOAD: pkl_dumps_lz4(deque()),
-        }
-    ],  # empty
-    indirect=True,
-)
 @pytest.mark.parametrize(
     "state_machine_input",
     [StateMachineInput.update(changelog_number_start=1, changelog_number_end=1)],
     indirect=True,
 )
 def test_changelog_number_update(
-    worker_data,
-    state_machine_input: StateMachineInput,
-    step_functions_client,
-    s3_client,
+    state_machine_input: StateMachineInput, step_functions_client, s3_client
 ):
-    execute_state_machine(state_machine_input, client=step_functions_client)
+    execute_state_machine(state_machine_input, step_functions_client)
     changelog_number_from_s3 = get_changelog_number_from_s3(s3_client)
     assert changelog_number_from_s3 == state_machine_input.changelog_number_end
 
@@ -193,7 +172,7 @@ def test_end_to_end(
     state_machine_input,
     step_functions_client,
 ):
-    execute_state_machine(state_machine_input, client=step_functions_client)
+    execute_state_machine(state_machine_input, step_functions_client)
 
     extract_data = get_object(s3_client, key=WorkerKey.EXTRACT)
     transform_data = pkl_loads_lz4(get_object(s3_client, key=WorkerKey.TRANSFORM))
@@ -217,9 +196,7 @@ def _wait_until_empty(get_data_fn, sleep_time_seconds=15):
 
 @long_running
 @pytest.mark.integration
-def test_end_to_end_bulk_trigger(
-    repository: MockDeviceRepository, step_functions_client, s3_client
-):
+def test_end_to_end_bulk_trigger(repository: MockDeviceRepository, s3_client):
     bucket = read_terraform_output("sds_etl.value.bucket")
     test_data_bucket = read_terraform_output("test_data_bucket.value")
     bulk_trigger_prefix = read_terraform_output("sds_etl.value.bulk_trigger_prefix")
@@ -291,7 +268,7 @@ def test_end_to_end_changelog_delete(
     state_machine_input,
 ):
     """Note that the start of this test is the same as test_end_to_end, and then makes changes"""
-    execute_state_machine(state_machine_input, client=step_functions_client)
+    execute_state_machine(state_machine_input, step_functions_client)
 
     extract_data = get_object(s3_client, key=WorkerKey.EXTRACT)
     transform_data = pkl_loads_lz4(get_object(s3_client, key=WorkerKey.TRANSFORM))
@@ -310,7 +287,7 @@ def test_end_to_end_changelog_delete(
         state_machine_input=StateMachineInput.update(
             changelog_number_start=124, changelog_number_end=125
         ),
-        client=step_functions_client,
+        step_functions_client=step_functions_client,
     )
     assert response["status"] == "SUCCEEDED"
 
@@ -336,7 +313,7 @@ def test_end_to_end_changelog_delete(
         state_machine_input=StateMachineInput.update(
             changelog_number_start=124, changelog_number_end=125
         ),
-        client=step_functions_client,
+        step_functions_client=step_functions_client,
     )
     assert response["status"] == "SUCCEEDED"
 

src/etl/sds/worker/load/tests/test_load_worker.py

@@ -1,6 +1,6 @@
 import os
 from collections import deque
-from datetime import datetime
+from datetime import datetime, timezone
 from itertools import chain, permutations
 from typing import Callable, Generator
 from unittest import mock
@@ -34,7 +34,7 @@
         "product_team_id": str(UUID(int=1)),
         "ods_code": "ABC",
         "status": "active",
-        "created_on": datetime.utcnow(),
+        "created_on": datetime.now(timezone.utc),
         "updated_on": None,
         "deleted_on": None,
     }
@@ -47,7 +47,7 @@
         "product_team_id": str(UUID(int=2)),
         "ods_code": "ABC",
         "status": "active",
-        "created_on": datetime.utcnow(),
+        "created_on": datetime.now(timezone.utc),
         "updated_on": None,
         "deleted_on": None,
     }

src/layers/domain/core/device.py

@@ -1,5 +1,5 @@
 from collections import defaultdict
-from datetime import datetime
+from datetime import datetime, timezone
 from enum import StrEnum, auto
 from itertools import chain
 from typing import Any, Optional
@@ -168,13 +168,13 @@ class Device(AggregateRoot):
 
     def update(self, **kwargs) -> DeviceUpdatedEvent:
         if "updated_on" not in kwargs:
-            kwargs["updated_on"] = datetime.utcnow()
+            kwargs["updated_on"] = datetime.now(timezone.utc)
         device_data = self._update(data=kwargs)
         event = DeviceUpdatedEvent(**device_data)
         return self.add_event(event)
 
     def delete(self) -> DeviceUpdatedEvent:
-        deletion_datetime = datetime.utcnow()
+        deletion_datetime = datetime.now(timezone.utc)
         return self.update(
             status=DeviceStatus.INACTIVE,
             updated_on=deletion_datetime,