Merge pull request #407 from NHSDigital/release/2024-11-13

Release/2024-11-13

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 2024-11-13
+- [PI-617] Stream bulk LDIF into blocks of party key
+- [PI-527] Swagger fix
+- [PI-592] GSI Upgrade
+- [PI-590] Read/write by alias
+
 ## 2024-11-08
 - [PI-508] Search DeviceReferenceData
 - [PI-578] Create MHS Device

README.md

@@ -10,12 +10,17 @@
    3. [AWS SSO Setup](#aws-sso-setup)
    4. [Other helpful commands](#other-helpful-commands)
 2. [Tests](#tests)
-3. [pytest tests](#pytest-tests)
-4. [End-to-End feature tests](#end-to-end-feature-tests)
-5. [Generate the Feature Test Postman collection](#generate-the-feature-test-postman-collection)
-6. [Workflow](#workflow)
-7. [Swagger](#swagger)
-8. [ETL](#etl)
+   1. [pytest tests](#pytest-tests)
+   2. [End-to-End feature tests](#end-to-end-feature-tests)
+   3. [Generate the Feature Test Postman collection](#generate-the-feature-test-postman-collection)
+3. [Data modelling](#data-modelling)
+   1. [Domain models](#domain-models)
+   2. [Database models](#database-models)
+   3. [Response models](#response-models)
+   4. [Request models](#request-models)
+4. [Workflow](#workflow)
+5. [Swagger](#swagger)
+6. [ETL](#etl)
 
 ---
 
@@ -273,6 +278,53 @@ in the environment (but these are filled out already if generated with the integ
 
 💡 **The feature tests are only guaranteed to work out-of-the-box with an empty database**
 
+## Data modelling
+
+Modelling in Connecting Party Manager is split into four partially-decoupled components:
+
+- Domain models: The conceptual entities of Connecting Party Manager, without any reference to database indexing, and request / response syntax.
+- Database models: A wrapper on the domain models that we persist to database (DynamoDB), indicating write-integrity (primary keys) and a read/search interface (indexing).
+- Response models: Deviations from the domain model (error handling, search result wrapping, private field exclusion, etc)
+- Request models: API-specific models indicating the expected request bodies of create/update/search operations.
+
+### Domain models
+
+TBC
+
+### Database models
+
+#### Write-integrity (primary keys)
+
+The Partition Key (`pk`) that we use for all objects\* in the database is a unique combination of prefix (aligned with the object type, e.g. `D#` for `Device`) and identifier (generally a UUID). The Sort Key (`sk`) that we use is always exactly equal to the Partition Key. This is opposed to having fully denormalised objects so that attributes are nested under their own `sk`. The reason for doing this is to limit multiple read operations for a given object, and also save I/O in our ETL process by reducing the number of database transactions required per object.
+
+Objects can additionally be indexed by any keys (see [Domain models](#domain-models)) that they have. For every key in an domain object,
+a copy is made in the database with the index being that key, rather
+than the object's identifier. Such copies are referred to as non-root
+objects, whereas the "original" (indexed by identifier) is referred to
+as the root object.
+
+\* In the case of `Device` tags, which sit outside of the standard database model, `pk` is equal to a query string and `sk` is equal to `pk` of the object that is referred to. A tag-indexed `Device` is otherwise a copy of the root `Device`.
+
+#### Read/search interface
+
+We have implemented a Global Secondary Index (GSI) for attributes named `pk_read` and `sk_read`. The pattern that is place is as follows:
+
+- `pk_read`: A concatenation of parent `pk` values (joined with `#`, e.g. `PT#<product_team_id>#P<product_id>`)
+- `sk_read`: Equal to the `pk` of the object itself.
+
+We refer to this as an "ownership" model, as it allows for reads to be
+executed in a way that mirrors the API read operations (GET `grandparent/parent/child`), whilst also giving us the ability to return all objects owned by the object indicated in the `pk_read` - which is a common operation for us.
+
+A `read` and `search` is available on all `Repository` patterns (almost) for free (the base `_read` and `_search` require a shallow wrapper, but most of the work is done for you).
+
+### Response models
+
+TBC
+
+### Request models
+
+TBC
+
 ## Workflow
 
 In order to create new branches, use the commands listed below. Note that the commands will throw an error if

VERSION

@@ -1 +1 @@
-2024.11.08
+2024.11.13

changelog/2024-11-13.md

@@ -0,0 +1,4 @@
+- [PI-617] Stream bulk LDIF into blocks of party key
+- [PI-527] Swagger fix
+- [PI-592] GSI Upgrade
+- [PI-590] Read/write by alias

infrastructure/swagger/08_components--schemas--other.yaml

@@ -3,7 +3,7 @@ components:
   schemas:
     HeaderVersion:
       type: string
-      pattern: "^[1-9][0-9]+$"
+      pattern: "^[1-9][0-9]?(\\.[0-9])?$"
 
     HeaderRequestId:
       type: string

infrastructure/terraform/per_workspace/main.tf

@@ -43,23 +43,15 @@ module "table" {
   attributes = [
     { name = "pk", type = "S" },
     { name = "sk", type = "S" },
-    { name = "pk_1", type = "S" },
-    { name = "sk_1", type = "S" },
-    { name = "pk_2", type = "S" },
-    { name = "sk_2", type = "S" }
+    { name = "pk_read", type = "S" },
+    { name = "sk_read", type = "S" },
   ]
 
   global_secondary_indexes = [
     {
-      name            = "idx_gsi_1"
-      hash_key        = "pk_1"
-      range_key       = "sk_1"
-      projection_type = "ALL"
-    },
-    {
-      name            = "idx_gsi_2"
-      hash_key        = "pk_2"
-      range_key       = "sk_2"
+      name            = "idx_gsi_read"
+      hash_key        = "pk_read"
+      range_key       = "sk_read"
       projection_type = "ALL"
     }
   ]

pyproject.toml

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

src/api/createCpmProductForEpr/tests/test_index.py

@@ -66,7 +66,7 @@ def test_index():
             table_name=TABLE_NAME, dynamodb_client=index.cache["DYNAMODB_CLIENT"]
         )
         read_product = repo.read(
-            product_team_id=product_team.id, product_id=created_product["id"]
+            product_team_id=product_team.id, id=created_product["id"]
         ).state()
 
     assert created_product == read_product

src/api/createDevice/tests/test_index.py

@@ -83,15 +83,19 @@ def test_index() -> None:
         assert device.name == DEVICE_NAME
         assert device.ods_code == ODS_CODE
         assert device.created_on.date() == datetime.today().date()
-        assert device.updated_on is None
-        assert device.deleted_on is None
+        assert not device.updated_on
+        assert not device.deleted_on
 
         # Retrieve the created resource
         repo = DeviceRepository(
             table_name=TABLE_NAME, dynamodb_client=index.cache["DYNAMODB_CLIENT"]
         )
 
-        created_device = repo.read(device.id)
+        created_device = repo.read(
+            product_team_id=device.product_team_id,
+            product_id=device.product_id,
+            id=device.id,
+        )
         assert created_device == device
 
 

src/api/createDeviceMessageHandlingSystem/tests/test_index.py

@@ -136,7 +136,7 @@ def test_index() -> None:
         assert device.ods_code == ODS_CODE
         assert device.created_on.date() == datetime.today().date()
         assert device.updated_on.date() == datetime.today().date()
-        assert device.deleted_on is None
+        assert not device.deleted_on
 
         questionnaire_responses = device.questionnaire_responses["spine_mhs/1"]
         assert len(questionnaire_responses) == 1
@@ -147,7 +147,11 @@ def test_index() -> None:
         repo = DeviceRepository(
             table_name=TABLE_NAME, dynamodb_client=index.cache["DYNAMODB_CLIENT"]
         )
-        created_device = repo.read(device.id)
+        created_device = repo.read(
+            product_team_id=device.product_team_id,
+            product_id=device.product_id,
+            id=device.id,
+        )
 
         # Check party_key is added to tags in the created device
         expected_party_key = (str(ProductKeyType.PARTY_KEY), "abc1234-987654")

src/api/createDeviceReferenceData/tests/test_index.py

@@ -96,7 +96,7 @@ def test_index() -> None:
         created_device_reference_data = repo.read(
             product_team_id=device_reference_data.product_team_id,
             product_id=device_reference_data.product_id,
-            device_reference_data_id=device_reference_data.id,
+            id=device_reference_data.id,
         )
         assert created_device_reference_data == device_reference_data
 

src/api/createDeviceReferenceDataASActions/tests/test_index.py

@@ -98,7 +98,7 @@ def test_index_without_questionnaire() -> None:
         created_device_reference_data = repo.read(
             product_team_id=device_reference_data.product_team_id,
             product_id=device_reference_data.product_id,
-            device_reference_data_id=device_reference_data.id,
+            id=device_reference_data.id,
         )
         assert created_device_reference_data == device_reference_data
 
@@ -155,6 +155,6 @@ def test_index_with_questionnaire() -> None:
         created_device_reference_data = repo.read(
             product_team_id=device_reference_data.product_team_id,
             product_id=device_reference_data.product_id,
-            device_reference_data_id=device_reference_data.id,
+            id=device_reference_data.id,
         )
         assert created_device_reference_data == device_reference_data

src/api/createDeviceReferenceDataMessageSet/tests/test_index.py

@@ -98,7 +98,7 @@ def test_index_without_questionnaire() -> None:
         created_device_reference_data = repo.read(
             product_team_id=device_reference_data.product_team_id,
             product_id=device_reference_data.product_id,
-            device_reference_data_id=device_reference_data.id,
+            id=device_reference_data.id,
         )
         assert created_device_reference_data == device_reference_data
 
@@ -157,6 +157,6 @@ def test_index_with_questionnaire() -> None:
         created_device_reference_data = repo.read(
             product_team_id=device_reference_data.product_team_id,
             product_id=device_reference_data.product_id,
-            device_reference_data_id=device_reference_data.id,
+            id=device_reference_data.id,
         )
         assert created_device_reference_data == device_reference_data

src/api/deleteCpmProduct/tests/test_index.py

@@ -4,14 +4,14 @@
 from unittest import mock
 
 import pytest
-from domain.core.cpm_product.v1 import CpmProduct
 from domain.core.cpm_system_id.v1 import ProductId
 from domain.core.enum import Status
 from domain.core.root.v3 import Root
-from domain.repository.cpm_product_repository.v3 import CpmProductRepository
+from domain.repository.cpm_product_repository.v3 import (
+    CpmProductRepository,
+    InactiveCpmProductRepository,
+)
 from domain.repository.errors import ItemNotFound
-from domain.repository.keys.v3 import TableKey
-from domain.repository.marshall import marshall, unmarshall
 from domain.repository.product_team_repository.v2 import ProductTeamRepository
 
 from test_helpers.dynamodb import mock_table
@@ -27,26 +27,6 @@
 VERSION = 1
 
 
-class MockCpmProductRepository(CpmProductRepository):
-    def read_inactive_product(
-        self, product_team_id: str, product_id: str
-    ) -> CpmProduct:
-        pk = TableKey.CPM_PRODUCT_STATUS.key(Status.INACTIVE, product_team_id)
-        sk = TableKey.CPM_PRODUCT.key(product_id)
-        args = {
-            "TableName": self.table_name,
-            "KeyConditionExpression": "pk = :pk AND sk = :sk",
-            "ExpressionAttributeValues": marshall(**{":pk": pk, ":sk": sk}),
-        }
-        result = self.client.query(**args)
-        items = [unmarshall(i) for i in result["Items"]]
-        if len(items) == 0:
-            raise ItemNotFound(product_team_id, product_id, item_type=CpmProduct)
-        (item,) = items
-
-        return CpmProduct(**item)
-
-
 @contextmanager
 def mock_lambda():
     org = Root.create_ods_organisation(ods_code=CPM_PRODUCT_TEAM_NO_ID["ods_code"])
@@ -100,13 +80,13 @@ def test_index():
             table_name=TABLE_NAME, dynamodb_client=index.cache["DYNAMODB_CLIENT"]
         )
         with pytest.raises(ItemNotFound):
-            repo.read(product_team_id=product_team.id, product_id=PRODUCT_ID)
+            repo.read(product_team_id=product_team.id, id=PRODUCT_ID)
 
-        repo = MockCpmProductRepository(
+        repo = InactiveCpmProductRepository(
             table_name=TABLE_NAME, dynamodb_client=index.cache["DYNAMODB_CLIENT"]
         )
-        deleted_product = repo.read_inactive_product(
-            product_team_id=product_team.id, product_id=PRODUCT_ID
+        deleted_product = repo.read(
+            product_team_id=product_team.id, id=PRODUCT_ID
         ).dict()
 
     # Sense checks on the deleted resource

src/api/readDevice/src/v1/steps.py

@@ -28,21 +28,30 @@ def read_product_team(data, cache) -> ProductTeam:
 
 def read_product(data, cache) -> CpmProduct:
     path_params: DevicePathParams = data[parse_path_params]
+    product_team: ProductTeam = data[read_product_team]
+
     product_repo = CpmProductRepository(
         table_name=cache["DYNAMODB_TABLE"], dynamodb_client=cache["DYNAMODB_CLIENT"]
     )
     cpm_product = product_repo.read(
-        product_id=path_params.product_id, product_team_id=path_params.product_team_id
+        product_team_id=product_team.id, id=path_params.product_id
     )
     return cpm_product
 
 
 def read_device(data, cache) -> Device:
     path_params: DevicePathParams = data[parse_path_params]
+    product_team: ProductTeam = data[read_product_team]
+    product: CpmProduct = data[read_product]
+
     device_repo = DeviceRepository(
         table_name=cache["DYNAMODB_TABLE"], dynamodb_client=cache["DYNAMODB_CLIENT"]
     )
-    return device_repo.read(path_params.device_id)
+    return device_repo.read(
+        product_team_id=product_team.id,
+        product_id=product.id,
+        id=path_params.device_id,
+    )
 
 
 def device_to_dict(data, cache) -> tuple[str, dict]:

src/api/readDevice/tests/test_index.py

@@ -154,7 +154,7 @@ def test_index_no_such_device(version):
             "errors": [
                 {
                     "code": "RESOURCE_NOT_FOUND",
-                    "message": "Could not find Device for key ('does not exist')",  # device saved by pk, sk = device.id still
+                    "message": f"Could not find Device for key ('{product_team.id}', 'P.XXX-YYY', 'does not exist')",
                 }
             ],
         }

src/api/readDeviceReferenceData/src/v1/steps.py

@@ -30,24 +30,30 @@ def read_product_team(data, cache) -> ProductTeam:
 
 def read_product(data, cache) -> CpmProduct:
     path_params: DeviceReferenceDataPathParams = data[parse_path_params]
+    product_team: ProductTeam = data[read_product_team]
+
     product_repo = CpmProductRepository(
         table_name=cache["DYNAMODB_TABLE"], dynamodb_client=cache["DYNAMODB_CLIENT"]
     )
     cpm_product = product_repo.read(
-        product_id=path_params.product_id, product_team_id=path_params.product_team_id
+        product_team_id=product_team.id,
+        id=path_params.product_id,
     )
     return cpm_product
 
 
 def read_device_reference_data(data, cache) -> DeviceReferenceData:
     path_params: DeviceReferenceDataPathParams = data[parse_path_params]
-    product_repo = DeviceReferenceDataRepository(
+    product_team: ProductTeam = data[read_product_team]
+    product: CpmProduct = data[read_product]
+
+    device_reference_data_repo = DeviceReferenceDataRepository(
         table_name=cache["DYNAMODB_TABLE"], dynamodb_client=cache["DYNAMODB_CLIENT"]
     )
-    return product_repo.read(
-        product_id=path_params.product_id,
-        product_team_id=path_params.product_team_id,
-        device_reference_data_id=path_params.device_reference_data_id,
+    return device_reference_data_repo.read(
+        product_team_id=product_team.id,
+        product_id=product.id,
+        id=path_params.device_reference_data_id,
     )