diff --git a/.tool-versions b/.tool-versions index e6b411675e..8bdd5e18b9 100644 --- a/.tool-versions +++ b/.tool-versions @@ -9,4 +9,4 @@ python 3.11.14 shellcheck 0.11.0 terraform 1.14.6 terraform-docs 0.20.0 -trivy 0.69.2 \ No newline at end of file +trivy 0.69.2 diff --git a/Makefile b/Makefile index f8093725b3..3bb820b7d2 100644 --- a/Makefile +++ b/Makefile @@ -124,14 +124,14 @@ download-api-certs: ## Downloads mTLS certificates (use with dev envs only). Usa rm -rf ./lambdas/mtls_env_certs/$(WORKSPACE) ./scripts/aws/download-api-certs.sh $(WORKSPACE) -test-lg-fhir-api-e2e: ## Runs LG FHIR API E2E tests. See readme for required environment variables. Usage: make test-fhir-api-e2e-lg CONTAINER= +test-lg-fhir-api-e2e: ## Runs LG FHIR API E2E tests. See readme for required environment variables. Usage: make test-lg-fhir-api-e2e CONTAINER= ifeq ($(CONTAINER), true) cd ./lambdas && PYTHONPATH=. poetry run pytest tests/e2e/api --ignore=tests/e2e/api/fhir -vv else cd ./lambdas && ./venv/bin/python3 -m pytest tests/e2e/api --ignore=tests/e2e/api/fhir -vv endif -test-core-fhir-api-e2e: guard-WORKSPACE ## Runs Core FHIR API E2E tests. Usage: make test-fhir-api-e2e-core WORKSPACE= CONTAINER= +test-core-fhir-api-e2e: guard-WORKSPACE ## Runs Core FHIR API E2E tests. Usage: make test-core-fhir-api-e2e WORKSPACE= CONTAINER= ./scripts/test/run-e2e-fhir-api-tests.sh --workspace $(WORKSPACE) --container $(CONTAINER) rm -rf ./lambdas/mtls_env_certs/$(WORKSPACE) diff --git a/apim/specification.yaml b/apim/specification.yaml index edc13fccd4..3a90ee30a6 100644 --- a/apim/specification.yaml +++ b/apim/specification.yaml @@ -177,14 +177,14 @@ info: version: 1.0.0 contact: name: NHS Digital API Management - url: 'https://digital.nhs.uk/developer/help-and-support' + url: "https://digital.nhs.uk/developer/help-and-support" email: api.management@nhs.net servers: - - url: 'https://sandbox.api.service.nhs.uk/national-document-repository/FHIR/R4' + - url: "https://sandbox.api.service.nhs.uk/national-document-repository/FHIR/R4" description: Sandbox environment. - - url: 'https://int.api.service.nhs.uk/national-document-repository/FHIR/R4' + - url: "https://int.api.service.nhs.uk/national-document-repository/FHIR/R4" description: Integration test environment. - - url: 'https://api.service.nhs.uk/national-document-repository/FHIR/R4' + - url: "https://api.service.nhs.uk/national-document-repository/FHIR/R4" description: Production environment. paths: /DocumentReference: @@ -356,41 +356,41 @@ paths: summary: Get a single document pointer operationId: readDocumentReference parameters: - - $ref: '#/components/parameters/id' - - $ref: '#/components/parameters/type' - - $ref: '#/components/parameters/requestId' - - $ref: '#/components/parameters/correlationId' + - $ref: "#/components/parameters/id" + - $ref: "#/components/parameters/type" + - $ref: "#/components/parameters/requestId" + - $ref: "#/components/parameters/correlationId" responses: - '200': + "200": description: Search successful response content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/DocumentReference' + $ref: "#/components/schemas/DocumentReference" example: resourceType: DocumentReference status: current docStatus: final type: coding: - - system: 'http://snomed.info/sct' - code: '16521000000101' + - system: "http://snomed.info/sct" + code: "16521000000101" display: Lloyd George record folder subject: identifier: - system: 'https://fhir.nhs.uk/Id/nhs-number' - value: '4409815415' - date: '2022-12-22T11:45:41+11:00' + system: "https://fhir.nhs.uk/Id/nhs-number" + value: "4409815415" + date: "2022-12-22T11:45:41+11:00" author: - identifier: - system: 'https://fhir.nhs.uk/Id/ods-organization-code' + system: "https://fhir.nhs.uk/Id/ods-organization-code" value: Organization/Y05868 authenticator: identifier: value: Organization/Y05868 custodian: identifier: - system: 'https://fhir.nhs.uk/Id/ods-organization-code' + system: "https://fhir.nhs.uk/Id/ods-organization-code" value: Y05868 content: - attachment: @@ -398,14 +398,14 @@ paths: language: en-US url: pre-signed url size: 100 - title: '1_of_1[example].pdf' - creation: '2022-12-22T09:45:41+11:00' - '400': + title: "1_of_1[example].pdf" + creation: "2022-12-22T09:45:41+11:00" + "400": description: Invalid request, missing id path parameter content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -413,16 +413,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'invalid' - display: 'Invaild' - diagnostics: 'Invalid request' - '401': + - system: "http://hl7.org/fhir/issue-type" + code: "invalid" + display: "Invaild" + diagnostics: "Invalid request" + "401": description: User has no auth, missing token content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -430,16 +430,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'unknown' - display: 'Unknown' - diagnostics: 'The user was not able to be authenticated' - '403': + - system: "http://hl7.org/fhir/issue-type" + code: "unknown" + display: "Unknown" + diagnostics: "The user was not able to be authenticated" + "403": description: User is unauthorised to view record content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -447,16 +447,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'forbidden' - display: 'Forbidden' - diagnostics: 'User is unauthorised to view record' - '404': + - system: "http://hl7.org/fhir/issue-type" + code: "forbidden" + display: "Forbidden" + diagnostics: "User is unauthorised to view record" + "404": description: ID provided does not match any document held content: application/fhir+json;verson=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -464,17 +464,16 @@ paths: code: exception details: coding: - - system: 'http://hl7.org/fhir/issue-type' - code: 'not-found' - display: 'Not Found' - diagnostics: 'Document reference not found' - + - system: "http://hl7.org/fhir/issue-type" + code: "not-found" + display: "Not Found" + diagnostics: "Document reference not found" headers: X-Correlation-Id: - $ref: '#/components/headers/CorrelationId' + $ref: "#/components/headers/CorrelationId" X-Request-Id: - $ref: '#/components/headers/RequestId' + $ref: "#/components/headers/RequestId" 4XX: description: | @@ -493,7 +492,7 @@ paths: content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" example: resourceType: OperationOutcome issue: @@ -501,8 +500,8 @@ paths: code: value details: coding: - - system: 'https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode' - version: '1' + - system: "https://fhir.nhs.uk/CodeSystem/NHSD-API-ErrorOrWarningCode" + version: "1" code: ACCESS_DENIED display: A parameter or value has resulted in a validation error diagnostics: The requested document cannot be read because it belongs to another organisation @@ -519,7 +518,7 @@ components: content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/DocumentReference' + $ref: "#/components/schemas/DocumentReference" required: true schemas: OperationOutcome: @@ -532,26 +531,26 @@ components: id: type: string pattern: '[A-Za-z0-9\-\.]{1,64}' - description: 'The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes.' + description: "The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes." meta: - $ref: '#/components/schemas/Meta' + $ref: "#/components/schemas/Meta" description: The metadata about the resource. This is content that is maintained by the infrastructure. Changes to the content might not always be associated with version changes to the resource. implicitRules: type: string pattern: \S* - description: 'A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc.' + description: "A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc." language: type: string pattern: '[^\s]+(\s[^\s]+)*' description: The base language in which the resource is written. text: - $ref: '#/components/schemas/Narrative' + $ref: "#/components/schemas/Narrative" description: 'A human–readable narrative that contains a summary of the resource and can be used to represent the content of the resource to a human. The narrative need not encode all the structured data, but is required to contain sufficient detail to make it "clinically safe" for a human to just read the narrative. Resource definitions may define what content should be represented in the narrative to ensure clinical safety.' issue: type: array items: - $ref: '#/components/schemas/OperationOutcomeIssue' - description: 'An error, warning, or information message that results from a system action.' + $ref: "#/components/schemas/OperationOutcomeIssue" + description: "An error, warning, or information message that results from a system action." minItems: 1 required: - resourceType @@ -570,9 +569,9 @@ components: code: type: string pattern: '[^\s]+(\s[^\s]+)*' - description: 'Describes the type of the issue. The system that creates an OperationOutcome SHALL choose the most applicable code from the IssueType value set, and may additional provide its own code for the error in the details element.' + description: "Describes the type of the issue. The system that creates an OperationOutcome SHALL choose the most applicable code from the IssueType value set, and may additional provide its own code for the error in the details element." details: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: Additional details about the error. This may be a text description of the error or a system code that identifies the error. diagnostics: type: string @@ -592,7 +591,7 @@ components: items: type: string pattern: '[ \r\n\t\S]+' - description: 'A [simple subset of FHIRPath](fhirpath.html#simple) limited to element names, repetition indicators and the default child accessor that identifies one of the elements in the resource that caused this issue to be raised.' + description: "A [simple subset of FHIRPath](fhirpath.html#simple) limited to element names, repetition indicators and the default child accessor that identifies one of the elements in the resource that caused this issue to be raised." required: - severity - code @@ -605,30 +604,30 @@ components: - DocumentReference id: type: string - pattern: '16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' - description: 'The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes.' + pattern: "16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" + description: "The logical id of the resource, as used in the URL for the resource. Once assigned, this value never changes." meta: - $ref: '#/components/schemas/Meta' + $ref: "#/components/schemas/Meta" description: The metadata about the resource. This is content that is maintained by the infrastructure. Changes to the content might not always be associated with version changes to the resource. implicitRules: type: string pattern: \S* - description: 'A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc.' + description: "A reference to a set of rules that were followed when the resource was constructed, and which must be understood when processing the content. Often, this is a reference to an implementation guide that defines the special rules along with other profiles etc." language: type: string pattern: '[^\s]+(\s[^\s]+)*' description: The base language in which the resource is written. text: - $ref: '#/components/schemas/Narrative' + $ref: "#/components/schemas/Narrative" description: 'A human–readable narrative that contains a summary of the resource and can be used to represent the content of the resource to a human. The narrative need not encode all the structured data, but is required to contain sufficient detail to make it "clinically safe" for a human to just read the narrative. Resource definitions may define what content should be represented in the narrative to ensure clinical safety.' masterIdentifier: - $ref: '#/components/schemas/Identifier' + $ref: "#/components/schemas/Identifier" description: Document identifier as assigned by the source of the document. This identifier is specific to this version of the document. This unique identifier may be used elsewhere to identify this version of the document. identifier: type: array items: - $ref: '#/components/schemas/Identifier' - description: 'Other identifiers associated with the document, including version independent identifiers.' + $ref: "#/components/schemas/Identifier" + description: "Other identifiers associated with the document, including version independent identifiers." status: type: string pattern: '[^\s]+(\s[^\s]+)*' @@ -638,16 +637,16 @@ components: pattern: '[^\s]+(\s[^\s]+)*' description: The status of the underlying document. type: - $ref: '#/components/schemas/CodeableConcept' - description: 'Specifies the particular kind of document referenced (e.g. History and Physical, Discharge Summary, Progress Note). This usually equates to the purpose of making the document referenced.' + $ref: "#/components/schemas/CodeableConcept" + description: "Specifies the particular kind of document referenced (e.g. History and Physical, Discharge Summary, Progress Note). This usually equates to the purpose of making the document referenced." category: type: array items: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: A categorization for the type of document referenced – helps for indexing and searching. This may be implied by or derived from the code specified in the DocumentReference.type. subject: - $ref: '#/components/schemas/Reference' - description: 'Who or what the document is about. The document can be about a person, (patient or healthcare practitioner), a device (e.g. a machine) or even a group of subjects (such as a document about a herd of farm animals, or a set of patients that share a common exposure).' + $ref: "#/components/schemas/Reference" + description: "Who or what the document is about. The document can be about a person, (patient or healthcare practitioner), a device (e.g. a machine) or even a group of subjects (such as a document about a herd of farm animals, or a set of patients that share a common exposure)." date: type: string pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00))' @@ -655,18 +654,18 @@ components: author: type: array items: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Identifies the Organisation or group who is responsible for adding the information to the document. authenticator: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Which person or organization authenticates that this document is valid. custodian: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Identifies the organization or group who is responsible for ongoing maintenance of and access to the document. relatesTo: type: array items: - $ref: '#/components/schemas/DocumentReferenceRelatesTo' + $ref: "#/components/schemas/DocumentReferenceRelatesTo" description: Relationships that this document has with other document references that already exist. description: type: string @@ -675,16 +674,16 @@ components: securityLabel: type: array items: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: 'A set of Security–Tag codes specifying the level of privacy/security of the Document. Note that DocumentReference.meta.security contains the security labels of the "reference" to the document, while DocumentReference.securityLabel contains a snapshot of the security labels on the document the reference refers to.' content: type: array items: - $ref: '#/components/schemas/DocumentReferenceContent' - description: 'The document and format referenced. There may be multiple content element repetitions, each with a different format.' + $ref: "#/components/schemas/DocumentReferenceContent" + description: "The document and format referenced. There may be multiple content element repetitions, each with a different format." minItems: 1 context: - $ref: '#/components/schemas/DocumentReferenceContext' + $ref: "#/components/schemas/DocumentReferenceContext" description: The clinical context in which the document was prepared. required: - resourceType @@ -875,29 +874,29 @@ components: encounter: type: array items: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Describes the clinical encounter or type of care that the document content is associated with. event: type: array items: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: 'This list of codes represents the main clinical acts, such as a colonoscopy or an appendectomy, being documented. In some cases, the event is inherent in the type Code, such as a "History and Physical Report" in which the procedure being documented is necessarily a "History and Physical" act.' period: - $ref: '#/components/schemas/Period' + $ref: "#/components/schemas/Period" description: The time period over which the service that is described by the document was provided. facilityType: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: The kind of facility where the patient was seen. practiceSetting: - $ref: '#/components/schemas/CodeableConcept' - description: 'This property may convey specifics about the practice setting where the content was created, often reflecting the clinical specialty.' + $ref: "#/components/schemas/CodeableConcept" + description: "This property may convey specifics about the practice setting where the content was created, often reflecting the clinical specialty." sourcePatientInfo: - $ref: '#/components/schemas/Reference' - description: 'The Patient Information as known when the document was published. May be a reference to a version specific, or contained.' + $ref: "#/components/schemas/Reference" + description: "The Patient Information as known when the document was published. May be a reference to a version specific, or contained." related: type: array items: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Related identifiers or resources associated with the DocumentReference. DocumentReferenceContent: type: object @@ -907,11 +906,11 @@ components: pattern: '[A-Za-z0-9\-\.]{1,64}' description: Unique id for the element within a resource (for internal references). This may be any string value that does not contain spaces. attachment: - $ref: '#/components/schemas/Attachment' + $ref: "#/components/schemas/Attachment" description: The document or URL of the document along with critical metadata to prove content has integrity. format: - $ref: '#/components/schemas/Coding' - description: 'An identifier of the document encoding, structure, and template that the document conforms to beyond the base format indicated in the mimeType.' + $ref: "#/components/schemas/Coding" + description: "An identifier of the document encoding, structure, and template that the document conforms to beyond the base format indicated in the mimeType." required: - attachment DocumentReferenceRelatesTo: @@ -926,7 +925,7 @@ components: pattern: '[^\s]+(\s[^\s]+)*' description: The type of relationship that this document has with anther document. target: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: The target document of this relationship. required: - code @@ -949,7 +948,7 @@ components: data: type: string pattern: '(\s*([0-9a-zA-Z\+/=]){4}\s*)+' - description: 'The actual data of the attachment – a sequence of bytes, base64 encoded.' + description: "The actual data of the attachment – a sequence of bytes, base64 encoded." url: type: string pattern: \S* @@ -957,7 +956,7 @@ components: size: type: integer format: int32 - description: 'The number of bytes of data that make up this attachment (before base64 encoding, if that is done).' + description: "The number of bytes of data that make up this attachment (before base64 encoding, if that is done)." hash: type: string pattern: '(\s*([0-9a-zA-Z\+/=]){4}\s*)+' @@ -980,7 +979,7 @@ components: coding: type: array items: - $ref: '#/components/schemas/Coding' + $ref: "#/components/schemas/Coding" description: A reference to a code defined by a terminology system. text: type: string @@ -1000,7 +999,7 @@ components: version: type: string pattern: '[ \r\n\t\S]+' - description: 'The version of the code system which was used when choosing this code. Note that a well–maintained code system does not need the version reported, because the meaning of codes is consistent across versions. However this cannot consistently be assured, and when the meaning is not guaranteed to be consistent, the version SHOULD be exchanged.' + description: "The version of the code system which was used when choosing this code. Note that a well–maintained code system does not need the version reported, because the meaning of codes is consistent across versions. However this cannot consistently be assured, and when the meaning is not guaranteed to be consistent, the version SHOULD be exchanged." code: type: string pattern: '[^\s]+(\s[^\s]+)*' @@ -1008,7 +1007,7 @@ components: display: type: string pattern: '[ \r\n\t\S]+' - description: 'A representation of the meaning of the code in the system, following the rules of the system.' + description: "A representation of the meaning of the code in the system, following the rules of the system." userSelected: type: boolean description: Indicates that this coding was chosen by a user directly – e.g. off a pick list of available items (codes or displays). @@ -1024,21 +1023,21 @@ components: pattern: '[^\s]+(\s[^\s]+)*' description: The purpose of this identifier. type: - $ref: '#/components/schemas/CodeableConcept' + $ref: "#/components/schemas/CodeableConcept" description: A coded type for the identifier that can be used to determine which identifier to use for a specific purpose. system: type: string pattern: \S* - description: 'Establishes the namespace for the value – that is, a URL that describes a set values that are unique.' + description: "Establishes the namespace for the value – that is, a URL that describes a set values that are unique." value: type: string pattern: '[ \r\n\t\S]+' description: The portion of the identifier typically relevant to the user and which is unique within the context of the system. period: - $ref: '#/components/schemas/Period' + $ref: "#/components/schemas/Period" description: Time period during which identifier is/was valid for use. assigner: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: Organization that issued/manages the identifier. Period: type: object @@ -1054,7 +1053,7 @@ components: end: type: string pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)(-(0[1-9]|1[0-2])(-(0[1-9]|[1-2][0-9]|3[0-1])(T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00)))?)?)?' - description: 'The end of the period. If the end of the period is missing, it means no end was known or planned at the time the instance was created. The start may be in the past, and the end date in the future, which means that period is expected/planned to end at that time.' + description: "The end of the period. If the end of the period is missing, it means no end was known or planned at the time the instance was created. The start may be in the past, and the end date in the future, which means that period is expected/planned to end at that time." Quantity: type: object properties: @@ -1091,7 +1090,7 @@ components: reference: type: string pattern: '[ \r\n\t\S]+' - description: 'A reference to a location at which the other resource is found. The reference may be a relative reference, in which case it is relative to the service base URL, or an absolute URL that resolves to the location where the resource is found. The reference may be version specific or not. If the reference is not to a FHIR RESTful server, then it should be assumed to be version specific. Internal fragment references (start with ''#'') refer to contained resources.' + description: "A reference to a location at which the other resource is found. The reference may be a relative reference, in which case it is relative to the service base URL, or an absolute URL that resolves to the location where the resource is found. The reference may be version specific or not. If the reference is not to a FHIR RESTful server, then it should be assumed to be version specific. Internal fragment references (start with '#') refer to contained resources." type: type: string pattern: \S* @@ -1099,8 +1098,8 @@ components: The expected type of the target of the reference. If both Reference.type and Reference.reference are populated and Reference.reference is a FHIR URL, both SHALL be consistent. The type is the Canonical URL of Resource Definition that is the type this reference refers to. References are URLs that are relative to http://hl7.org/fhir/StructureDefinition/ e.g. "Patient" is a reference to http://hl7.org/fhir/StructureDefinition/Patient. Absolute URLs are only allowed for logical models (and can only be used in references in logical models, not resources). identifier: - $ref: '#/components/schemas/Identifier' - description: 'An identifier for the target resource. This is used when there is no way to reference the other resource directly, either because the entity it represents is not available through a FHIR server, or because there is no way for the author of the resource to convert a known identifier to an actual location. There is no requirement that a Reference.identifier point to something that is actually exposed as a FHIR instance, but it SHALL point to a business concept that would be expected to be exposed as a FHIR instance, and that instance would need to be of a FHIR resource type allowed by the reference.' + $ref: "#/components/schemas/Identifier" + description: "An identifier for the target resource. This is used when there is no way to reference the other resource directly, either because the entity it represents is not available through a FHIR server, or because there is no way for the author of the resource to convert a known identifier to an actual location. There is no requirement that a Reference.identifier point to something that is actually exposed as a FHIR instance, but it SHALL point to a business concept that would be expected to be exposed as a FHIR instance, and that instance would need to be of a FHIR resource type allowed by the reference." display: type: string pattern: '[ \r\n\t\S]+' @@ -1115,7 +1114,7 @@ components: type: type: array items: - $ref: '#/components/schemas/Coding' + $ref: "#/components/schemas/Coding" description: An indication of the reason that the entity signed this document. This may be explicitly included as part of the signature information and can be used when determining accountability for various actions concerning the document. minItems: 1 when: @@ -1123,10 +1122,10 @@ components: pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00))' description: When the digital signature was signed. who: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: A reference to an application–usable description of the identity that signed (e.g. the signature used their private key). onBehalfOf: - $ref: '#/components/schemas/Reference' + $ref: "#/components/schemas/Reference" description: A reference to an application–usable description of the identity that is represented by the signature. targetFormat: type: string @@ -1135,7 +1134,7 @@ components: sigFormat: type: string pattern: '[^\s]+(\s[^\s]+)*' - description: 'A mime type that indicates the technical format of the signature. Important mime types are application/signature+xml for X ML DigSig, application/jose for JWS, and image/* for a graphical image of a signature, etc.' + description: "A mime type that indicates the technical format of the signature. Important mime types are application/signature+xml for X ML DigSig, application/jose for JWS, and image/* for a graphical image of a signature, etc." data: type: string pattern: '(\s*([0-9a-zA-Z\+/=]){4}\s*)+' @@ -1154,7 +1153,7 @@ components: versionId: type: string pattern: '[A-Za-z0-9\-\.]{1,64}' - description: 'The version specific identifier, as it appears in the version portion of the URL. This value changes when the resource is created, updated, or deleted.' + description: "The version specific identifier, as it appears in the version portion of the URL. This value changes when the resource is created, updated, or deleted." lastUpdated: type: string pattern: '([0-9]([0-9]([0-9][1-9]|[1-9]0)|[1-9]00)|[1-9]000)-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])T([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\.[0-9]+)?(Z|(\+|-)((0[0-9]|1[0-3]):[0-5][0-9]|14:00))' @@ -1162,23 +1161,23 @@ components: source: type: string pattern: \S* - description: 'A uri that identifies the source system of the resource. This provides a minimal amount of [Provenance](provenance.html#) information that can be used to track or differentiate the source of information in the resource. The source may identify another FHIR server, document, message, database, etc.' + description: "A uri that identifies the source system of the resource. This provides a minimal amount of [Provenance](provenance.html#) information that can be used to track or differentiate the source of information in the resource. The source may identify another FHIR server, document, message, database, etc." profile: type: array items: type: string pattern: \S* - description: 'A list of profiles (references to [StructureDefinition](structuredefinition.html#) resources) that this resource claims to conform to. The URL is a reference to [StructureDefinition.url](structuredefinition–definitions.html#StructureDefinition.url).' + description: "A list of profiles (references to [StructureDefinition](structuredefinition.html#) resources) that this resource claims to conform to. The URL is a reference to [StructureDefinition.url](structuredefinition–definitions.html#StructureDefinition.url)." security: type: array items: - $ref: '#/components/schemas/Coding' + $ref: "#/components/schemas/Coding" description: Security labels applied to this resource. These tags connect specific resources to the overall security policy and infrastructure. tag: type: array items: - $ref: '#/components/schemas/Coding' - description: 'Tags applied to this resource. Tags are intended to be used to identify and relate resources to process and workflow, and applications are not required to consider the tags when interpreting the meaning of a resource.' + $ref: "#/components/schemas/Coding" + description: "Tags applied to this resource. Tags are intended to be used to identify and relate resources to process and workflow, and applications are not required to consider the tags when interpreting the meaning of a resource." Narrative: type: object properties: @@ -1189,71 +1188,71 @@ components: status: type: string pattern: '[^\s]+(\s[^\s]+)*' - description: 'The status of the narrative – whether it''s entirely generated (from just the defined data or the extensions too), or whether a human authored it and it may contain additional data.' + description: "The status of the narrative – whether it's entirely generated (from just the defined data or the extensions too), or whether a human authored it and it may contain additional data." div: type: string - description: 'The actual narrative content, a stripped down version of XHTML.' + description: "The actual narrative content, a stripped down version of XHTML." required: - status - div DocumentId: type: string - pattern: '16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}' + pattern: "16521000000101~[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[0-9a-f]{4}-[0-9a-f]{12}" RequestPathParams: type: object properties: id: - $ref: '#/components/schemas/DocumentId' + $ref: "#/components/schemas/DocumentId" required: - id RequestHeader: type: object properties: odsCode: - $ref: '#/components/schemas/RequestHeaderOdsCode' + $ref: "#/components/schemas/RequestHeaderOdsCode" required: - odsCode RequestParams: type: object properties: - 'subject:identifier': - $ref: '#/components/schemas/RequestQuerySubject' - 'custodian:identifier': - $ref: '#/components/schemas/RequestQueryCustodian' + "subject:identifier": + $ref: "#/components/schemas/RequestQuerySubject" + "custodian:identifier": + $ref: "#/components/schemas/RequestQueryCustodian" type: - $ref: '#/components/schemas/RequestQueryType' + $ref: "#/components/schemas/RequestQueryType" category: - $ref: '#/components/schemas/RequestQueryCategory' + $ref: "#/components/schemas/RequestQueryCategory" next-page-token: - $ref: '#/components/schemas/NextPageToken' + $ref: "#/components/schemas/NextPageToken" required: - - 'subject:identifier' + - "subject:identifier" CountRequestParams: type: object properties: - 'subject:identifier': - $ref: '#/components/schemas/RequestQuerySubject' + "subject:identifier": + $ref: "#/components/schemas/RequestQuerySubject" required: - - 'subject:identifier' + - "subject:identifier" RequestQuerySubject: type: string pattern: '^https\:\/\/fhir\.nhs\.uk\/Id\/nhs-number\|(\d+)$' - example: 'https://fhir.nhs.uk/Id/nhs-number|4409815415' + example: "https://fhir.nhs.uk/Id/nhs-number|4409815415" RequestQueryCustodian: type: string pattern: '^https\:\/\/fhir\.nhs\.uk\/Id\/ods-organization-code\|(\w+)$' - example: 'https://fhir.nhs.uk/Id/ods-organization-code|Y05868' + example: "https://fhir.nhs.uk/Id/ods-organization-code|Y05868" RequestQueryType: type: string - example: 'http://snomed.info/sct|736253002' + example: "http://snomed.info/sct|736253002" RequestQueryCategory: type: string - example: 'http://snomed.info/sct|734163000' + example: "http://snomed.info/sct|734163000" NextPageToken: type: string RequestHeaderRequestId: type: string - pattern: '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' + pattern: "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" example: 60E0B220-8136-4CA5-AE46-1D97EF59D068 RequestHeaderCorrelationId: type: string @@ -1265,7 +1264,7 @@ components: required: true description: logical identifier schema: - $ref: '#/components/schemas/DocumentId' + $ref: "#/components/schemas/DocumentId" examples: valid: summary: Valid ID, document held, user has correct auth (200) @@ -1283,24 +1282,24 @@ components: summary: Document not Found, {id} does not match any documents held. (404) value: 404 subject: - name: 'subject:identifier' + name: "subject:identifier" in: query schema: - $ref: '#/components/schemas/RequestQuerySubject' + $ref: "#/components/schemas/RequestQuerySubject" required: true examples: none: summary: None - value: '' + value: "" valid_1: - summary: 'Valid #1' - value: 'https://fhir.nhs.uk/Id/nhs-number|4409815415' + summary: "Valid #1" + value: "https://fhir.nhs.uk/Id/nhs-number|4409815415" valid_2: - summary: 'Valid #2' - value: 'https://fhir.nhs.uk/Id/nhs-number|3495456481' + summary: "Valid #2" + value: "https://fhir.nhs.uk/Id/nhs-number|3495456481" invalid: summary: Unknown - value: 'https://fhir.nhs.uk/Id/nhs-number|3495456001' + value: "https://fhir.nhs.uk/Id/nhs-number|3495456001" custodian: name: custodian:identifier in: query @@ -1320,17 +1319,17 @@ components: name: type:identifier in: query schema: - $ref: '#/components/schemas/RequestQueryType' + $ref: "#/components/schemas/RequestQueryType" examples: none: summary: None - value: '' + value: "" SNOMED_CODES_LLOYD_GEORGE_RECORD_FOLDER: summary: Lloyd George record folder - value: 'http://snomed.info/sct|16521000000101' + value: "http://snomed.info/sct|16521000000101" invalid: summary: Unknown - value: 'http://snomed.info/sct|410970009' + value: "http://snomed.info/sct|410970009" nextPageToken: name: next-page-token description: | @@ -1350,7 +1349,7 @@ components: Mirrored back in a response header. in: header schema: - $ref: '#/components/schemas/RequestHeaderRequestId' + $ref: "#/components/schemas/RequestHeaderRequestId" correlationId: name: X-Correlation-ID description: | @@ -1364,20 +1363,20 @@ components: in: header required: false schema: - $ref: '#/components/schemas/RequestHeaderCorrelationId' + $ref: "#/components/schemas/RequestHeaderCorrelationId" responses: Success: description: Success Response content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" Error: description: Error Response content: application/fhir+json;version=1: schema: - $ref: '#/components/schemas/OperationOutcome' + $ref: "#/components/schemas/OperationOutcome" headers: CorrelationId: schema: @@ -1388,7 +1387,7 @@ components: RequestId: schema: type: string - pattern: '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' + pattern: "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" example: 60E0B220-8136-4CA5-AE46-1D97EF59D068 description: | The X-Request-ID from the request header mirrored back. diff --git a/lambdas/handlers/get_fhir_document_reference_handler.py b/lambdas/handlers/get_fhir_document_reference_handler.py index 25df6ba44f..4b66c15756 100644 --- a/lambdas/handlers/get_fhir_document_reference_handler.py +++ b/lambdas/handlers/get_fhir_document_reference_handler.py @@ -1,5 +1,11 @@ -from enums.lambda_error import LambdaError +import uuid +from typing import Optional, Tuple + from oauthlib.oauth2 import WebApplicationClient + +from enums.lambda_error import LambdaError +from enums.mtls import MtlsCommonNames +from enums.snomed_codes import SnomedCodes from services.base.ssm_service import SSMService from services.dynamic_configuration_service import DynamicConfigurationService from services.get_fhir_document_reference_service import GetFhirDocumentReferenceService @@ -15,6 +21,7 @@ SearchPatientException, ) from utils.lambda_handler_utils import extract_bearer_token +from utils.lambda_header_utils import validate_common_name_in_mtls from utils.lambda_response import ApiGatewayResponse logger = LoggingService(__name__) @@ -29,44 +36,52 @@ "APPCONFIG_ENVIRONMENT", "PRESIGNED_ASSUME_ROLE", "CLOUDFRONT_URL", - ] + ], ) def lambda_handler(event, context): try: + common_name = validate_common_name_in_mtls(event.get("requestContext")) bearer_token = extract_bearer_token(event, context) selected_role_id = event.get("headers", {}).get("cis2-urid", None) document_id, snomed_code = extract_document_parameters(event) + if not snomed_code: + snomed_code = _determine_document_type(common_name=common_name) get_document_service = GetFhirDocumentReferenceService() document_reference = get_document_service.handle_get_document_reference_request( - snomed_code, document_id + snomed_code, + document_id, ) if selected_role_id and bearer_token: verify_user_authorisation( - bearer_token, selected_role_id, document_reference.nhs_number + bearer_token, + selected_role_id, + document_reference.nhs_number, ) document_reference_response = ( get_document_service.create_document_reference_fhir_response( - document_reference + document_reference, ) ) logger.info( - f"Successfully retrieved document reference for document_id: {document_id}, snomed_code: {snomed_code}" + f"Successfully retrieved document reference for document_id: {document_id}", ) return ApiGatewayResponse( - status_code=200, body=document_reference_response, methods="GET" + status_code=200, + body=document_reference_response, + methods="GET", ).create_api_gateway_response() except GetFhirDocumentReferenceException as exception: return ApiGatewayResponse( status_code=exception.status_code, body=exception.error.create_error_response().create_error_fhir_response( - exception.error.value.get("fhir_coding") + exception.error.value.get("fhir_coding"), ), methods="GET", ).create_api_gateway_response() @@ -75,12 +90,13 @@ def lambda_handler(event, context): def extract_document_parameters(event): """Extract document ID and SNOMED code from path parameters""" path_params = event.get("pathParameters", {}).get("id", None) - document_id, snomed_code = get_id_and_snomed_from_path_parameters(path_params) + document_id, snomed_code = get_id_from_path_parameters(path_params) - if not document_id or not snomed_code: - logger.error("Missing document id or snomed code in request path parameters.") + if not document_id: + logger.error("Missing document ID in request path parameters.") raise GetFhirDocumentReferenceException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) return document_id, snomed_code @@ -100,27 +116,72 @@ def verify_user_authorisation(bearer_token, selected_role_id, nhs_number): org_ods_code = oidc_service.fetch_user_org_code(userinfo, selected_role_id) smartcard_role_code, _ = oidc_service.fetch_user_role_code( - userinfo, selected_role_id, "R" + userinfo, + selected_role_id, + "R", ) except (OidcApiException, AuthorisationException) as e: logger.error(f"Authorization error: {str(e)}") raise GetFhirDocumentReferenceException( - 403, LambdaError.DocumentReferenceUnauthorised + 403, + LambdaError.DocumentReferenceUnauthorised, ) try: search_patient_service = SearchPatientDetailsService( - smartcard_role_code, org_ods_code + smartcard_role_code, + org_ods_code, ) search_patient_service.handle_search_patient_request(nhs_number, False) except SearchPatientException as e: raise GetFhirDocumentReferenceException(e.status_code, e.error) -def get_id_and_snomed_from_path_parameters(path_parameters): - """Extract document ID and SNOMED code from path parameters""" - if path_parameters: - params = path_parameters.split("~") - if len(params) == 2: - return params[1], params[0] - return None, None +def get_id_from_path_parameters(path_parameters) -> Tuple[Optional[str], Optional[str]]: + """Extract uuid from path parameters. + + Accepts: + - '1234~uuid' + - 'uuid' + """ + snomed_code = None + if not path_parameters: + return None, None + + params = path_parameters.split("~") + if len(params) > 2: + raise GetFhirDocumentReferenceException( + 400, + LambdaError.DocRefInvalidFiles, + ) + + if len(params) > 1: + snomed_code = params[0] if params[0] else None + doc_id = params[1] + else: + doc_id = params[-1] + if not is_uuid(doc_id): + raise GetFhirDocumentReferenceException( + 400, + LambdaError.DocRefInvalidFiles, + ) + return doc_id, snomed_code + + +def is_uuid(value: str) -> bool: + try: + uuid.UUID(value) + return True + except (ValueError, TypeError): + return False + + +def _determine_document_type(common_name: MtlsCommonNames | None) -> str: + if not common_name: + return SnomedCodes.LLOYD_GEORGE.value.code + + if common_name not in MtlsCommonNames: + logger.error(f"mTLS common name {common_name} - is not supported") + raise GetFhirDocumentReferenceException(400, LambdaError.DocRefInvalidType) + + return SnomedCodes.PATIENT_DATA.value.code diff --git a/lambdas/services/delete_fhir_document_reference_service.py b/lambdas/services/delete_fhir_document_reference_service.py index a1150fb953..122755b555 100644 --- a/lambdas/services/delete_fhir_document_reference_service.py +++ b/lambdas/services/delete_fhir_document_reference_service.py @@ -1,8 +1,9 @@ import uuid -from datetime import datetime, timezone from typing import Dict, Optional from botocore.exceptions import ClientError +from pydantic import ValidationError + from enums.document_retention import DocumentRetentionDays from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields @@ -10,14 +11,12 @@ from enums.snomed_codes import SnomedCode, SnomedCodes from enums.supported_document_types import SupportedDocumentTypes from models.document_reference import DocumentReference -from pydantic import ValidationError from services.document_deletion_service import DocumentDeletionService from services.document_service import DocumentService from services.fhir_document_reference_service_base import ( FhirDocumentReferenceServiceBase, ) from utils.audit_logging_setup import LoggingService -from utils.common_query_filters import NotDeleted from utils.exceptions import DynamoServiceException, InvalidNhsNumberException from utils.lambda_exceptions import ( DocumentDeletionServiceException, @@ -38,7 +37,8 @@ def __init__(self): super().__init__() def process_fhir_document_reference( - self, event: dict = {} + self, + event: dict = {}, ) -> list[DocumentReference]: """ Process a FHIR Document Reference DELETE request @@ -52,7 +52,8 @@ def process_fhir_document_reference( if any(v is None for v in deletion_identifiers): logger.error("FHIR document validation error: NhsNumber/id") raise DocumentRefException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) if len(deletion_identifiers) < 2: @@ -61,7 +62,8 @@ def process_fhir_document_reference( if not self.is_uuid(deletion_identifiers[0]): logger.error("FHIR document validation error: Id") raise DocumentRefException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) doc_type = self._determine_document_type_based_on_common_name(common_name) @@ -69,7 +71,8 @@ def process_fhir_document_reference( if not validate_nhs_number(deletion_identifiers[1]): logger.error("FHIR document validation error: NhsNumber") raise DocumentRefException( - 400, LambdaError.DocumentReferenceMissingParameters + 400, + LambdaError.DocumentReferenceMissingParameters, ) request_context.patient_nhs_no = deletion_identifiers[1] @@ -84,12 +87,9 @@ def process_fhir_document_reference( fhir=True, ) else: - files_deleted = ( - self.delete_fhir_document_reference_by_nhs_id_and_doc_id( - nhs_number=deletion_identifiers[1], - doc_id=deletion_identifiers[0], - doc_type=doc_type, - ) + files_deleted = self.delete_fhir_document_reference_by_doc_id( + doc_id=deletion_identifiers[0], + doc_type=doc_type, ) return files_deleted @@ -100,14 +100,15 @@ def process_fhir_document_reference( logger.error(f"AWS client error: {str(e)}") raise DocumentRefException(500, LambdaError.InternalServerError) - def delete_fhir_document_reference_by_nhs_id_and_doc_id( - self, nhs_number: str, doc_id: str, doc_type: SnomedCode + def delete_fhir_document_reference_by_doc_id( + self, + doc_id: str, + doc_type: SnomedCode, ) -> DocumentReference | None: dynamo_table = self._get_dynamo_table_for_doc_type(doc_type) document_service = DocumentService() document = document_service.get_item_agnostic( - partion_key={DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number}, - sort_key={DocumentReferenceMetadataFields.ID.value: doc_id}, + partition_key={DocumentReferenceMetadataFields.ID.value: doc_id}, table_name=dynamo_table, ) if not document: @@ -119,7 +120,6 @@ def delete_fhir_document_reference_by_nhs_id_and_doc_id( document_ttl_days=DocumentRetentionDays.SOFT_DELETE, key_pair={ DocumentReferenceMetadataFields.ID.value: doc_id, - DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number, }, ) logger.info( @@ -134,30 +134,38 @@ def delete_fhir_document_reference_by_nhs_id_and_doc_id( ) raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) - def delete_fhir_document_references_by_nhs_id( - self, nhs_number: str, doc_type: SnomedCode - ) -> list[DocumentReference] | None: + def delete_fhir_document_reference_by_nhs_id_and_doc_id( + self, + nhs_number: str, + doc_id: str, + doc_type: SnomedCode, + ) -> DocumentReference | None: dynamo_table = self._get_dynamo_table_for_doc_type(doc_type) document_service = DocumentService() - documents = document_service.fetch_documents_from_table( - search_condition=nhs_number, - search_key="NhsNumber", + document = document_service.get_item_agnostic( + partition_key={ + DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number, + }, + sort_key={DocumentReferenceMetadataFields.ID.value: doc_id}, table_name=dynamo_table, - query_filter=NotDeleted, ) - if not documents: + if not document: return None try: - document_service.delete_document_references( + document_service.delete_document_reference( table_name=dynamo_table, - document_references=documents, + document_reference=document, document_ttl_days=DocumentRetentionDays.SOFT_DELETE, + key_pair={ + DocumentReferenceMetadataFields.ID.value: doc_id, + DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number, + }, ) logger.info( f"Deleted document of type {doc_type.display_name}", {"Result": "Successful deletion"}, ) - return documents + return document except (ClientError, DynamoServiceException) as e: logger.error( f"{LambdaError.DocDelClient.to_str()}: {str(e)}", @@ -166,7 +174,8 @@ def delete_fhir_document_references_by_nhs_id( raise DocumentDeletionServiceException(500, LambdaError.DocDelClient) def _determine_document_type_based_on_common_name( - self, common_name: MtlsCommonNames | None + self, + common_name: MtlsCommonNames | None, ) -> SnomedCode: if not common_name: """Determine the document type based on common_name""" @@ -186,7 +195,7 @@ def is_uuid(self, value: str) -> bool: def extract_parameters(self, event) -> list[str]: nhs_id, document_reference_id = self.extract_document_query_parameters( - event.get("queryStringParameters") or {} + event.get("queryStringParameters") or {}, ) if not nhs_id or not document_reference_id: @@ -201,7 +210,8 @@ def extract_document_path_parameters(self, event): return doc_ref_id def extract_document_query_parameters( - self, query_string: Dict[str, str] + self, + query_string: Dict[str, str], ) -> tuple[Optional[str], Optional[str]]: nhs_number = None document_reference_id = None diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 2fe77fdbc8..f4f91ca50c 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -108,6 +108,7 @@ def _search_tables_for_documents( documents = self.fetch_documents_from_table( search_condition=nhs_number, search_key="NhsNumber", + index_name="idx_gsi_nhs_number", table_name=table_name, query_filter=filter_expression, ) diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 32a2927d18..f0ebd042e4 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -1,6 +1,6 @@ import os from datetime import datetime, timezone -from typing import Optional +from typing import List, Optional from boto3.dynamodb.conditions import Attr, ConditionBase from botocore.exceptions import ClientError @@ -106,24 +106,41 @@ def fetch_documents_from_table( continue return documents - def _get_item(self, table_name, key, model_class): + def filter_item(self, response, filters=[]): + if len(filters) > 0: + for filter in filters: + for key in filter: + attribute = response.get("Item").get(key, None) + if attribute != filter[key]: + return False + return True + + def _get_item(self, table_name, key, model_class, return_deleted=False, filters=[]): try: response = self.dynamo_service.get_item(table_name=table_name, key=key) if "Item" not in response: logger.info("No document found") return None - document = model_class.model_validate(response["Item"]) - return document + if not return_deleted: + deleted = response.get("Item").get("Deleted", None) + if deleted not in (None, ""): + return None + + if self.filter_item(response, filters): + document = model_class.model_validate(response.get("Item")) + return document + + return None except ValidationError as e: - logger.error(f"Validation error on document: {response.get('Item')}") + logger.error(f"Validation error on document: {response.get('Item') or ''}") logger.error(f"{e}") return None def get_item_agnostic( self, - partion_key: dict, + partition_key: dict, sort_key: dict | None = None, table_name: str | None = None, model_class: type[BaseModel] | None = None, @@ -133,7 +150,7 @@ def get_item_agnostic( return self._get_item( table_name=table_name, - key=(partion_key or {}) | (sort_key or {}), + key=(partition_key or {}) | (sort_key or {}), model_class=model_class, ) @@ -143,6 +160,8 @@ def get_item( sort_key: dict | None = None, table_name: str = None, model_class: type[BaseModel] = None, + return_deleted: bool = False, + filters: List[dict] = [], ) -> Optional[BaseModel]: """Fetch a single document by ID from a specified or configured table. @@ -164,6 +183,8 @@ def get_item( table_name=table_to_use, key=document_key, model_class=model_to_use, + return_deleted=return_deleted, + filters=filters, ) def get_nhs_numbers_based_on_ods_code( diff --git a/lambdas/services/get_fhir_document_reference_service.py b/lambdas/services/get_fhir_document_reference_service.py index 183bbc3175..e3f3f6469d 100644 --- a/lambdas/services/get_fhir_document_reference_service.py +++ b/lambdas/services/get_fhir_document_reference_service.py @@ -24,7 +24,7 @@ def __init__(self): get_document_presign_url_aws_role_arn = os.getenv("PRESIGNED_ASSUME_ROLE") self.cloudfront_url = os.environ.get("CLOUDFRONT_URL") self.s3_service = S3Service( - custom_aws_role=get_document_presign_url_aws_role_arn + custom_aws_role=get_document_presign_url_aws_role_arn, ) self.ssm_service = SSMService() self.document_service = DocumentService() @@ -37,7 +37,8 @@ def handle_get_document_reference_request(self, snomed_code, document_id): document_reference = self.get_document_references(document_id, dynamo_table) else: document_reference = self.get_core_document_references( - document_id=document_id, snomed_code=snomed_code, table=dynamo_table + document_id=document_id, + table=dynamo_table, ) return document_reference @@ -47,27 +48,32 @@ def _get_dynamo_table_for_doc_type(self, doc_type: SnomedCode) -> str: return self.doc_router.resolve(doc_type) except KeyError: logger.error( - f"SNOMED code {doc_type.code} - {doc_type.display_name} is not supported" + f"SNOMED code {doc_type.code} - {doc_type.display_name} is not supported", ) raise GetFhirDocumentReferenceException(400, LambdaError.DocTypeInvalid) def get_document_references(self, document_id: str, table) -> DocumentReference: return self.fetch_documents( - search_key="ID", search_condition=document_id, table=table + search_key="ID", + search_condition=document_id, + table=table, ) def get_core_document_references( - self, document_id: str, snomed_code: str, table - ) -> DocumentReference: - index_name = "idx_gsi_snomed_code" - search_key = ["DocumentSnomedCodeType", "ID"] - search_condition = [snomed_code, document_id] - return self.fetch_documents( - search_key=search_key, - search_condition=search_condition, - index_name=index_name, - table=table, + self, + document_id: str, + table, + ) -> DocumentReference | None: + document_reference = self.document_service.get_item( + document_id=document_id, + table_name=table, ) + if not document_reference: + raise GetFhirDocumentReferenceException( + 404, + LambdaError.DocumentReferenceNotFound, + ) + return document_reference def fetch_documents( self, @@ -86,10 +92,10 @@ def fetch_documents( if len(documents) > 0: logger.info("Document found for given id") return documents[0] - else: - raise GetFhirDocumentReferenceException( - 404, LambdaError.DocumentReferenceNotFound - ) + raise GetFhirDocumentReferenceException( + 404, + LambdaError.DocumentReferenceNotFound, + ) def get_presigned_url(self, bucket_name, file_location): """ @@ -109,7 +115,8 @@ def get_presigned_url(self, bucket_name, file_location): return presign_url_response def create_document_reference_fhir_response( - self, document_reference: DocumentReference + self, + document_reference: DocumentReference, ) -> str: """ Creates a FHIR-compliant DocumentReference response for a given document. @@ -161,7 +168,7 @@ def create_document_reference_fhir_response( custodian=document_reference.current_gp_ods, attachment=document_details, snomed_code_doc_type=SnomedCodes.find_by_code( - document_reference.document_snomed_code_type + document_reference.document_snomed_code_type, ), ) .create_fhir_document_reference_object(document_reference) diff --git a/lambdas/services/upload_document_reference_service.py b/lambdas/services/upload_document_reference_service.py index 564df4b083..722d90ffc0 100644 --- a/lambdas/services/upload_document_reference_service.py +++ b/lambdas/services/upload_document_reference_service.py @@ -58,14 +58,10 @@ def handle_upload_document_reference_request( try: object_parts = object_key.split("/") document_key = object_parts[-1] - nhs_number = None - if len(object_parts) > 1: - nhs_number = object_parts[-2] self._get_infrastructure_for_document_key(object_parts) preliminary_document_reference = self._fetch_preliminary_document_reference( document_key, - nhs_number, ) if not preliminary_document_reference: return @@ -102,43 +98,47 @@ def _get_infrastructure_for_document_key(self, object_parts: list[str]) -> None: def _fetch_preliminary_document_reference( self, document_key: str, - nhs_number: str | None = None, ) -> Optional[DocumentReference]: """Fetch document reference from the database""" try: if self.doc_type.code != SnomedCodes.PATIENT_DATA.value.code: search_key = "ID" search_condition = document_key - else: - if not nhs_number: + documents = self.document_service.fetch_documents_from_table( + search_key=search_key, + search_condition=search_condition, + table_name=self.table_name, + query_filter=PreliminaryStatus, + ) + if not documents: logger.error( - f"Failed to process object key with ID: {document_key}", + f"No document with the following key found in {self.table_name} table: {document_key}", ) - raise FileProcessingException(400, LambdaError.DocRefInvalidFiles) + logger.info("Skipping this object") + return None - search_key = ["NhsNumber", "ID"] - search_condition = [nhs_number, document_key] + if len(documents) > 1: + logger.warning( + f"Multiple documents found for key {document_key}, using first one", + ) - documents = self.document_service.fetch_documents_from_table( - search_key=search_key, - search_condition=search_condition, + return documents[0] + + document = self.document_service.get_item( + document_id=document_key, table_name=self.table_name, - query_filter=PreliminaryStatus, + return_deleted=False, + filters=[{"DocStatus": "preliminary"}], ) - if not documents: + if not document: logger.error( f"No document with the following key found in {self.table_name} table: {document_key}", ) logger.info("Skipping this object") return None - if len(documents) > 1: - logger.warning( - f"Multiple documents found for key {document_key}, using first one", - ) - - return documents[0] + return document except ClientError as e: logger.error( @@ -484,7 +484,6 @@ def _update_dynamo_table( if self.doc_type.code == SnomedCodes.PATIENT_DATA.value.code: update_fields.add("s3_file_key") update_key = { - DocumentReferenceMetadataFields.NHS_NUMBER.value: document.nhs_number, DocumentReferenceMetadataFields.ID.value: document.id, } diff --git a/lambdas/tests/e2e/api/fhir/conftest.py b/lambdas/tests/e2e/api/fhir/conftest.py index 891ac53f46..695fd49386 100644 --- a/lambdas/tests/e2e/api/fhir/conftest.py +++ b/lambdas/tests/e2e/api/fhir/conftest.py @@ -105,7 +105,7 @@ def get_pdm_document_reference( endpoint_override=None, ): if not endpoint_override: - url = f"https://{MTLS_ENDPOINT}/{resource_type}/{pdm_snomed}~{record_id}" + url = f"https://{MTLS_ENDPOINT}/{resource_type}/{record_id}" else: url = f"https://{MTLS_ENDPOINT}/{resource_type}/{endpoint_override}" headers = { diff --git a/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py b/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py index 03404131e3..0d9d76c570 100644 --- a/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_mtls_api_failure.py @@ -83,23 +83,6 @@ def test_unsupported_http_method_returns_fhir_error(http_method): ) -def test_5xx_response_is_fhir_compliant(test_data): - """Verify that a Lambda error returns a FHIR-compliant OperationOutcome via DEFAULT_5XX.""" - - reversed_id = f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}" - - response = get_pdm_document_reference( - endpoint_override=reversed_id, - ) - - assert response.status_code == 500 - body = response.json() - assert body["resourceType"] == "OperationOutcome" - assert len(body["issue"]) > 0 - issue = body["issue"][0] - assert issue["severity"] == "error" - - def test_mtls_invalid_common_name(): record_id = str(uuid.uuid4()) response = get_pdm_document_reference( diff --git a/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py b/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py index e5a4df3f9f..457a369b54 100644 --- a/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py +++ b/lambdas/tests/e2e/api/fhir/test_retrieve_document_fhir_api_failure.py @@ -76,11 +76,11 @@ def test_retrieve_non_existant_document_reference( @pytest.mark.parametrize( "param,expected_status,expected_code", [ - (f"{pdm_data_helper.snomed_code}-{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}+{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}&{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{pdm_data_helper.snomed_code}{str(uuid.uuid4())}", 400, "MISSING_VALUE"), - (f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}", 500, "exception"), + (f"{pdm_data_helper.snomed_code}-{str(uuid.uuid4())}", 400, "invalid"), + (f"{pdm_data_helper.snomed_code}+{str(uuid.uuid4())}", 400, "invalid"), + (f"{pdm_data_helper.snomed_code}&{str(uuid.uuid4())}", 400, "invalid"), + (f"{pdm_data_helper.snomed_code}{str(uuid.uuid4())}", 400, "invalid"), + (f"{str(uuid.uuid4())}~{pdm_data_helper.snomed_code}", 400, "invalid"), ], ) def test_incorrectly_formatted_path_param_id( @@ -98,18 +98,6 @@ def test_incorrectly_formatted_path_param_id( _assert_operation_outcome(body=body, code=expected_code) -def test_no_snomed_code_in_path_param_id(test_data): - pdm_record = create_and_store_pdm_record(test_data) - response = get_pdm_document_reference( - record_id=pdm_record["id"], - pdm_snomed="", - ) - - body = response.json() - assert response.status_code == 400 - _assert_operation_outcome(body=body, code="MISSING_VALUE") - - def test_no_document_id_in_path_param_id(): response = get_pdm_document_reference() @@ -137,4 +125,4 @@ def test_extra_parameter_in_id_in_path_param_id(test_data): body = response.json() assert response.status_code == 400 - _assert_operation_outcome(body=body, code="MISSING_VALUE") + _assert_operation_outcome(body=body, code="invalid") diff --git a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py index d3eedc8c46..82fdb0c05f 100644 --- a/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py +++ b/lambdas/tests/e2e/api/fhir/test_upload_document_fhir_api_success.py @@ -4,7 +4,6 @@ import os from tests.e2e.api.fhir.conftest import ( - PDM_SNOMED, TEST_NHS_NUMBER, retrieve_document_with_retry, upload_document, @@ -47,7 +46,7 @@ def test_create_document_base64(test_data): # Validate attachment URL attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{upload_response['id']}" in attachment_url ) @@ -96,7 +95,7 @@ def test_create_document_base64_medium_file(test_data): attachment_url = upload_response["content"][0]["attachment"]["url"] assert ( - f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{PDM_SNOMED}~" + f"https://{APIM_ENDPOINT}/national-document-repository/FHIR/R4/DocumentReference/{upload_response['id']}" in attachment_url ) diff --git a/lambdas/tests/e2e/api/test_search_patient_api.py b/lambdas/tests/e2e/api/test_search_patient_api.py index 2a28349c66..69c13b1dd1 100644 --- a/lambdas/tests/e2e/api/test_search_patient_api.py +++ b/lambdas/tests/e2e/api/test_search_patient_api.py @@ -4,6 +4,7 @@ import requests from syrupy.filters import paths + from tests.e2e.conftest import API_ENDPOINT, API_KEY, APIM_ENDPOINT, LLOYD_GEORGE_SNOMED from tests.e2e.helpers.data_helper import LloydGeorgeDataHelper diff --git a/lambdas/tests/e2e/helpers/data_helper.py b/lambdas/tests/e2e/helpers/data_helper.py index aa708b821b..c91fb46911 100644 --- a/lambdas/tests/e2e/helpers/data_helper.py +++ b/lambdas/tests/e2e/helpers/data_helper.py @@ -216,22 +216,6 @@ def __init__(self): "pdm_record", ) - def retrieve_document_reference(self, record): - return self.dynamo_service.get_item( - table_name=self.dynamo_table, - key={"NhsNumber": record["nhs_number"], "ID": record["id"]}, - ) - - def tidyup(self, record): - self.dynamo_service.delete_item( - table_name=self.dynamo_table, - key={"NhsNumber": record["nhs_number"], "ID": record["id"]}, - ) - self.s3_service.delete_object( - self.s3_bucket, - f"{record['nhs_number']}/{record['id']}", - ) - class LloydGeorgeDataHelper(DataHelper): def __init__(self): diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index 75fd49077a..750ebf9861 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -68,7 +68,7 @@ MOCK_STATISTICAL_REPORTS_BUCKET_ENV_NAME = "STATISTICAL_REPORTS_BUCKET" MOCK_ARF_TABLE_NAME = "test_arf_dynamoDB_table" -MOCK_PDM_TABLE_NAME = "test_pdm_dynamoDB_table" +MOCK_PDM_TABLE_NAME = "COREDocumentMetadata" MOCK_LG_TABLE_NAME = "test_lg_dynamoDB_table" MOCK_UNSTITCHED_LG_TABLE_NAME = "test_unstitched_lg_table" MOCK_BULK_REPORT_TABLE_NAME = "test_report_dynamoDB_table" @@ -84,7 +84,7 @@ MOCK_STATISTICS_REPORT_BUCKET_NAME = "test_statistics_report_bucket" REVIEW_SQS_QUEUE_URL = "test_review_queue" TEST_NHS_NUMBER = "9000000009" -TEST_UUID = "1234-4567-8912-HSDF-TEST" +TEST_UUID = "550e8400-e29b-41d4-a716-446655440000" TEST_FILE_KEY = "test_file_key" TEST_FILE_NAME = "test.pdf" TEST_FILE_SIZE = 24000 diff --git a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py index cdd3e6f873..feb5a76967 100644 --- a/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_get_fhir_document_reference_handler.py @@ -2,11 +2,12 @@ from copy import deepcopy import pytest + from enums.lambda_error import LambdaError from enums.snomed_codes import SnomedCodes from handlers.get_fhir_document_reference_handler import ( extract_document_parameters, - get_id_and_snomed_from_path_parameters, + get_id_from_path_parameters, lambda_handler, verify_user_authorisation, ) @@ -40,7 +41,7 @@ MOCK_EVENT_APPLICATION["headers"] = {"Authorization": f"Bearer {TEST_UUID}"} MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) @@ -58,7 +59,7 @@ def mock_oidc_service(mocker): @pytest.fixture def mock_config_service(mocker): mock_config = mocker.patch( - "handlers.get_fhir_document_reference_handler.DynamicConfigurationService" + "handlers.get_fhir_document_reference_handler.DynamicConfigurationService", ) mock_config_instance = mock_config.return_value return mock_config_instance @@ -67,7 +68,7 @@ def mock_config_service(mocker): @pytest.fixture def mock_document_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService" + "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService", ) mock_service_instance = mock_service.return_value mock_service_instance.handle_get_document_reference_request.return_value = ( @@ -79,7 +80,7 @@ def mock_document_service(mocker): @pytest.fixture def mock_search_patient_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.SearchPatientDetailsService" + "handlers.get_fhir_document_reference_handler.SearchPatientDetailsService", ) mock_service_instance = mock_service.return_value return mock_service_instance @@ -106,13 +107,15 @@ def test_lambda_handler_happy_path_with_cis2_login( mock_oidc_service.fetch_user_org_code.assert_called_once() mock_oidc_service.fetch_user_role_code.assert_called_once() mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_search_patient_service.handle_search_patient_request.assert_called_once_with( - "9000000009", False + "9000000009", + False, ) mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -137,11 +140,12 @@ def test_lambda_handler_happy_path_with_application_login( mock_oidc_service.fetch_user_org_code.assert_not_called() mock_oidc_service.fetch_user_role_code.assert_not_called() mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_search_patient_service.handle_search_patient_request.assert_not_called() mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -167,14 +171,17 @@ def test_extract_document_parameters_valid(): def test_extract_document_parameters_invalid(): - with pytest.raises(GetFhirDocumentReferenceException) as e: - extract_document_parameters(MOCK_INVALID_EVENT_ID_MALFORMED) - assert e.value.status_code == 400 - assert e.value.error == LambdaError.DocumentReferenceMissingParameters + document_id, snomed_code = extract_document_parameters( + MOCK_INVALID_EVENT_ID_MALFORMED, + ) + assert document_id == TEST_UUID + assert snomed_code is None def test_verify_user_authorisation( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): try: verify_user_authorisation(f"Bearer {TEST_UUID}", TEST_UUID, "9000000009") @@ -183,7 +190,9 @@ def test_verify_user_authorisation( def test_verify_user_authorization_raise_oidc_error( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): mock_oidc_service.fetch_userinfo.side_effect = OidcApiException("OIDC error") with pytest.raises(GetFhirDocumentReferenceException) as excinfo: @@ -193,7 +202,9 @@ def test_verify_user_authorization_raise_oidc_error( def test_verify_user_authorization_raise_search_patient_error( - mock_oidc_service, mock_config_service, mock_search_patient_service + mock_oidc_service, + mock_config_service, + mock_search_patient_service, ): mock_search_patient_service.handle_search_patient_request.side_effect = ( SearchPatientException(403, LambdaError.MockError) @@ -205,7 +216,11 @@ def test_verify_user_authorization_raise_search_patient_error( def test_lambda_handler_missing_auth( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context + set_env, + mock_oidc_service, + mock_config_service, + mock_document_service, + context, ): event_without_auth = deepcopy(MOCK_CIS2_VALID_EVENT) event_without_auth["headers"] = {} @@ -215,16 +230,12 @@ def test_lambda_handler_missing_auth( mock_document_service.handle_get_document_reference_request.assert_not_called() -def test_lambda_handler_id_malformed( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context -): - response = lambda_handler(MOCK_INVALID_EVENT_ID_MALFORMED, context) - assert response["statusCode"] == 400 - mock_document_service.handle_get_document_reference_request.assert_not_called() - - def test_lambda_handler_oidc_error( - set_env, mock_config_service, mock_document_service, context, mocker + set_env, + mock_config_service, + mock_document_service, + context, + mocker, ): mocker.patch( "handlers.get_fhir_document_reference_handler.OidcService.set_up_oidc_parameters", @@ -238,7 +249,11 @@ def test_lambda_handler_oidc_error( def test_lambda_handler_invalid_path_parameters( - set_env, mock_oidc_service, mock_config_service, mock_document_service, context + set_env, + mock_oidc_service, + mock_config_service, + mock_document_service, + context, ): event_with_invalid_path = deepcopy(MOCK_CIS2_VALID_EVENT) event_with_invalid_path["pathParameters"] = {"id": "invalid_format_no_tilde"} @@ -312,34 +327,42 @@ def test_lambda_handler_search_service_errors( == LambdaError.MockError.value.get("fhir_coding").display ) assert response_body["issue"][0]["diagnostics"] == LambdaError.MockError.value.get( - "message" + "message", ) -def test_get_id_and_snomed_from_path_parameters(): - path_parameter = f"{SNOMED_CODE}~{TEST_UUID}" - - document_id, snomed = get_id_and_snomed_from_path_parameters(path_parameter) +@pytest.mark.parametrize( + "path_parameter", + [ + (f"{SNOMED_CODE}~{TEST_UUID}"), + (f"~{TEST_UUID}"), + (f"{TEST_UUID}"), + ], +) +def test_get_id_from_path_parameters(path_parameter): + document_id, snomed_code = get_id_from_path_parameters(path_parameter) assert document_id == TEST_UUID - assert snomed == SNOMED_CODE - - -def test_get_id_and_snomed_from_path_parameters_no_tilde(): - document_id, snomed = get_id_and_snomed_from_path_parameters("notildePresent") - assert document_id is None - assert snomed is None - + if snomed_code: + assert isinstance(snomed_code, str) + else: + assert snomed_code is None -def test_get_id_and_snomed_from_path_parameters_extra_tildes(): - # Test with extra tildes - path_parameter = f"{SNOMED_CODE}~{TEST_UUID}~extra" - document_id, snomed = get_id_and_snomed_from_path_parameters(path_parameter) - assert document_id is None - assert snomed is None +@pytest.mark.parametrize( + "path_parameter", + [ + ("notauuid"), + (f"{SNOMED_CODE}~{TEST_UUID}~extra"), + ], +) +def test_get_id_from_path_parameters_raises_error(path_parameter): + with pytest.raises(GetFhirDocumentReferenceException) as excinfo: + get_id_from_path_parameters(path_parameter) + assert excinfo.value.status_code == 400 + assert excinfo.value.error == LambdaError.DocRefInvalidFiles -def test_get_id_and_snomed_from_path_parameters_empty(): - document_id, snomed = get_id_and_snomed_from_path_parameters("") +def test_get_id_from_path_parameters_empty(): + document_id, snomed_code = get_id_from_path_parameters("") assert document_id is None - assert snomed is None + assert snomed_code is None diff --git a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py index ad1521349f..147e7b0891 100644 --- a/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py +++ b/lambdas/tests/unit/handlers/test_pdm_get_fhir_document_reference_by_id_handler.py @@ -1,4 +1,5 @@ import pytest + from enums.mtls import MtlsCommonNames from enums.snomed_codes import SnomedCodes from handlers.get_fhir_document_reference_handler import ( @@ -15,7 +16,7 @@ MOCK_MTLS_VALID_EVENT = { "httpMethod": "GET", "headers": {}, - "pathParameters": {"id": f"{SNOMED_CODE}~{TEST_UUID}"}, + "pathParameters": {"id": f"{TEST_UUID}"}, "body": None, "requestContext": { "accountId": "123456789012", @@ -39,14 +40,14 @@ } MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) @pytest.fixture def mock_config_service(mocker): mock_config = mocker.patch( - "handlers.get_fhir_document_reference_handler.DynamicConfigurationService" + "handlers.get_fhir_document_reference_handler.DynamicConfigurationService", ) mock_config_instance = mock_config.return_value return mock_config_instance @@ -55,7 +56,7 @@ def mock_config_service(mocker): @pytest.fixture def mock_document_service(mocker): mock_service = mocker.patch( - "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService" + "handlers.get_fhir_document_reference_handler.GetFhirDocumentReferenceService", ) mock_service_instance = mock_service.return_value mock_service_instance.handle_get_document_reference_request.return_value = ( @@ -89,10 +90,11 @@ def test_lambda_handler_happy_path_with_mtls_pdm_login( assert response["body"] == "test_document_reference" # Verify correct method calls mock_document_service.handle_get_document_reference_request.assert_called_once_with( - SNOMED_CODE, TEST_UUID + SNOMED_CODE, + TEST_UUID, ) mock_document_service.create_document_reference_fhir_response.assert_called_once_with( - MOCK_DOCUMENT_REFERENCE + MOCK_DOCUMENT_REFERENCE, ) @@ -103,5 +105,5 @@ def test_extract_bearer_token_when_pdm(context, mock_mtls_common_names): def test_extract_document_parameters_valid_pdm(): document_id, snomed_code = extract_document_parameters(MOCK_MTLS_VALID_EVENT) + assert snomed_code is None assert document_id == TEST_UUID - assert snomed_code == SNOMED_CODE diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py index a2fddb79c1..1418000b1a 100644 --- a/lambdas/tests/unit/services/test_document_service.py +++ b/lambdas/tests/unit/services/test_document_service.py @@ -723,3 +723,164 @@ def test_get_item_document_id_with_sort_key(mock_service, mock_dynamo_service): table_name=MOCK_LG_TABLE_NAME, key={"ID": document_id, **sort_key}, ) + + +@pytest.mark.parametrize( + "document_id,table_name,file_name,expected_table,return_deleted", + [ + ( + "test-doc-id-123", + MOCK_LG_TABLE_NAME, + "test-file.pdf", + MOCK_LG_TABLE_NAME, + False, + ), + ("test-doc-id-456", MOCK_TABLE_NAME, "lg-file.pdf", MOCK_TABLE_NAME, False), + ( + "test-doc-id-123", + MOCK_LG_TABLE_NAME, + "test-file.pdf", + MOCK_LG_TABLE_NAME, + True, + ), + ("test-doc-id-456", MOCK_TABLE_NAME, "lg-file.pdf", MOCK_TABLE_NAME, True), + ], +) +def test_get_item_returns_deleted( + mock_service, + mock_dynamo_service, + document_id, + table_name, + file_name, + expected_table, + return_deleted, +): + """Test successful retrieval of a document with default or custom table.""" + mock_dynamo_response = { + "Item": { + "ID": document_id, + "NhsNumber": TEST_NHS_NUMBER, + "FileName": file_name, + "Created": "2023-01-01T00:00:00Z", + "Deleted": "foobar", + "VirusScannerResult": "Clean", + }, + } + + mock_dynamo_service.get_item.return_value = mock_dynamo_response + + result = mock_service._get_item( + table_name=table_name, + key={"ID": document_id}, + model_class=DocumentReference, + return_deleted=return_deleted, + ) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=expected_table, + key={"ID": document_id}, + ) + if return_deleted: + assert result is not None + assert isinstance(result, DocumentReference) + assert result.id == document_id + assert result.nhs_number == TEST_NHS_NUMBER + else: + assert result is None + + +@pytest.mark.parametrize( + "filters", + [ + ([{"DocStatus": "preliminary"}]), + ([{"DocStatus": "current"}]), + ], +) +def test_filters_on_get_item( + mock_service, + filters, +): + mock_dynamo_response = { + "Item": { + "ID": "test-doc-id-123", + "NhsNumber": TEST_NHS_NUMBER, + "FileName": "test-file.pdf", + "Created": "2023-01-01T00:00:00Z", + "Deleted": "foobar", + "VirusScannerResult": "Clean", + "DocStatus": "preliminary", + }, + } + + result = mock_service.filter_item(response=mock_dynamo_response, filters=filters) + if filters[0].get("DocStatus") == "preliminary": + assert result is True + else: + assert result is False + + +@pytest.mark.parametrize( + "document_id,table_name,file_name,expected_table,return_deleted,filters", + [ + ( + "test-doc-id-123", + MOCK_LG_TABLE_NAME, + "test-file.pdf", + MOCK_LG_TABLE_NAME, + False, + [{"DocStatus": "preliminary"}], + ), + ( + "test-doc-id-456", + MOCK_TABLE_NAME, + "lg-file.pdf", + MOCK_TABLE_NAME, + False, + [{"DocStatus": "current"}], + ), + ], +) +def test_get_item_returns_filtered( + mock_service, + mock_dynamo_service, + document_id, + table_name, + file_name, + expected_table, + return_deleted, + filters, +): + """Test successful retrieval of a document with default or custom table.""" + mock_dynamo_response = { + "Item": { + "ID": document_id, + "NhsNumber": TEST_NHS_NUMBER, + "FileName": file_name, + "Created": "2023-01-01T00:00:00Z", + "Deleted": "", + "VirusScannerResult": "Clean", + "DocStatus": "preliminary", + }, + } + + mock_dynamo_service.get_item.return_value = mock_dynamo_response + + result = mock_service._get_item( + table_name=table_name, + key={"ID": document_id}, + model_class=DocumentReference, + return_deleted=return_deleted, + filters=filters, + ) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=expected_table, + key={"ID": document_id}, + ) + if filters[0].get("DocStatus") == "preliminary": + assert result is not None + assert isinstance(result, DocumentReference) + assert result.id == document_id + assert result.nhs_number == TEST_NHS_NUMBER + else: + assert result is None diff --git a/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py b/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py index 938f714e03..05d91fb49b 100644 --- a/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py +++ b/lambdas/tests/unit/services/test_pdm_delete_fhir_document_reference_by_nhs_id_service.py @@ -1,27 +1,25 @@ import uuid -from datetime import datetime, timezone from unittest.mock import MagicMock, patch -from lambdas.models import document_reference import pytest from botocore.exceptions import ClientError + from enums.document_retention import DocumentRetentionDays from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.mtls import MtlsCommonNames from enums.snomed_codes import SnomedCodes -from models.document_reference import DocumentReference, Identifier +from models.document_reference import DocumentReference from services.delete_fhir_document_reference_service import ( DeleteFhirDocumentReferenceService, ) from tests.unit.conftest import TEST_NHS_NUMBER from tests.unit.helpers.data.dynamo.dynamo_responses import MOCK_SEARCH_RESPONSE -from utils.common_query_filters import NotDeleted from utils.lambda_exceptions import ( DocumentRefException, ) MOCK_DOCUMENT_REFERENCE = DocumentReference.model_validate( - MOCK_SEARCH_RESPONSE["Items"][0] + MOCK_SEARCH_RESPONSE["Items"][0], ) TEST_DOCUMENT_ID = "3d8683b9-1665-40d2-8499-6e8302d507ff" @@ -73,7 +71,7 @@ "httpMethod": "DELETE", "headers": {}, "queryStringParameters": { - "foo": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}" + "foo": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}", }, "body": None, } @@ -99,7 +97,7 @@ "httpMethod": "DELETE", "headers": {}, "queryStringParameters": { - "subject:identifier": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}" + "subject:identifier": f"https://fhir.nhs.uk/Id/nhs-number|{TEST_NHS_NUMBER}", }, "pathParameters": {"id": f"{TEST_DOCUMENT_ID}"}, "body": None, @@ -125,7 +123,7 @@ def test_none(service): def test_extract_path_parameter_no_path_param_exists_for_id(service): identifier = service.extract_document_path_parameters( - MOCK_MTLS_VALID_EVENT_BY_NHS_ID + MOCK_MTLS_VALID_EVENT_BY_NHS_ID, ) assert identifier is None @@ -134,21 +132,21 @@ def test_extract_path_parameter_no_path_param_exists_for_id_but_pathParameters_e service, ): identifier = service.extract_document_path_parameters( - MOCK_MTLS_VALID_EVENT_WITH_INVALID_PATH_PARAM + MOCK_MTLS_VALID_EVENT_WITH_INVALID_PATH_PARAM, ) assert identifier is None def test_extract_path_parameter_path_param_exists_for_id(service): identifier = service.extract_document_path_parameters( - MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM + MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM, ) assert identifier is TEST_DOCUMENT_ID def test_extract_query_parameter_for_id_and_nhsnumber(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_BY_NHS_ID["queryStringParameters"] + MOCK_MTLS_VALID_EVENT_BY_NHS_ID["queryStringParameters"], ) assert identifiers[0] == TEST_NHS_NUMBER assert identifiers[1] == TEST_DOCUMENT_ID @@ -156,21 +154,21 @@ def test_extract_query_parameter_for_id_and_nhsnumber(service): def test_extract_query_parameter_with_invalid_query_parameter(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM["queryStringParameters"] + MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM["queryStringParameters"], ) assert identifiers == (None, None) def test_extract_query_parameter_when_non_existent(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM + MOCK_MTLS_VALID_EVENT_WITH_VALID_PATH_PARAM_NO_QUERY_PARAM, ) assert identifiers == (None, None) def test_extract_query_parameter_with_too_many(service): identifiers = service.extract_document_query_parameters( - MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM_AND_VALID_QUERY_PARAMS + MOCK_MTLS_VALID_EVENT_WITH_INVALID_QUERY_PARAM_AND_VALID_QUERY_PARAMS, ) assert identifiers == (None, None) @@ -190,7 +188,7 @@ def test_extract_parameters_when_no_query_and_no_path(service): def test_doc_type_by_common_name_pdm(service): doc_type = service._determine_document_type_based_on_common_name( - MtlsCommonNames.PDM + MtlsCommonNames.PDM, ) assert doc_type.code == SnomedCodes.PATIENT_DATA.value.code @@ -201,7 +199,6 @@ def test_doc_type_by_common_name_None(service): def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service): - nhs_number = "9000000009" doc_type = MagicMock() doc_type.value = "test-doc-type" @@ -213,15 +210,14 @@ def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service service.delete_document_reference = MagicMock() with patch( - "services.delete_fhir_document_reference_service.DocumentService" + "services.delete_fhir_document_reference_service.DocumentService", ) as mock_document_service_cls: mock_document_service = MagicMock() mock_document_service.get_item_agnostic.return_value = mock_document mock_document_service_cls.return_value = mock_document_service - result = service.delete_fhir_document_reference_by_nhs_id_and_doc_id( - nhs_number=nhs_number, + result = service.delete_fhir_document_reference_by_doc_id( doc_id=TEST_DOCUMENT_ID, doc_type=doc_type, ) @@ -231,8 +227,7 @@ def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service service._get_dynamo_table_for_doc_type.assert_called_once_with(doc_type) mock_document_service.get_item_agnostic.assert_called_once_with( - partion_key={DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number}, - sort_key={DocumentReferenceMetadataFields.ID.value: TEST_DOCUMENT_ID}, + partition_key={DocumentReferenceMetadataFields.ID.value: TEST_DOCUMENT_ID}, table_name=dynamo_table, ) @@ -241,7 +236,6 @@ def test_delete_fhir_document_references_by_nhs_id_and_doc_id_happy_path(service document_reference=mock_document, document_ttl_days=DocumentRetentionDays.SOFT_DELETE, key_pair={ - DocumentReferenceMetadataFields.NHS_NUMBER.value: nhs_number, DocumentReferenceMetadataFields.ID.value: TEST_DOCUMENT_ID, }, ) @@ -253,15 +247,14 @@ def test_delete_fhir_document_references_by_nhs_id_no_documents(service): service.delete_document_reference = MagicMock() with patch( - "services.delete_fhir_document_reference_service.DocumentService" + "services.delete_fhir_document_reference_service.DocumentService", ) as mock_document_service_cls: mock_document_service = MagicMock() mock_document_service.get_item_agnostic.return_value = [] mock_document_service_cls.return_value = mock_document_service - result = service.delete_fhir_document_reference_by_nhs_id_and_doc_id( - nhs_number="9000000009", + result = service.delete_fhir_document_reference_by_doc_id( doc_id=TEST_DOCUMENT_ID, doc_type=MagicMock(), ) @@ -274,12 +267,13 @@ def test_delete_fhir_document_references_by_nhs_id_propagates_client_error(servi service._get_dynamo_table_for_doc_type = MagicMock(return_value="mock-table") with patch( - "services.delete_fhir_document_reference_service.DocumentService" + "services.delete_fhir_document_reference_service.DocumentService", ) as mock_document_service_cls: mock_document_service = MagicMock() mock_document_service.get_item_agnostic.side_effect = ClientError( - error_response={}, operation_name="Query" + error_response={}, + operation_name="Query", ) mock_document_service_cls.return_value = mock_document_service @@ -291,42 +285,6 @@ def test_delete_fhir_document_references_by_nhs_id_propagates_client_error(servi ) -def test_process_calls_delete_by_nhs_id_for_pdm(service): - event = {"requestContext": {}} - deletion_identifiers = [TEST_DOCUMENT_ID, "9000000009"] - - with patch( - "services.delete_fhir_document_reference_service.validate_common_name_in_mtls", - return_value="CN", - ), patch.object( - service, - "extract_parameters", - return_value=deletion_identifiers, - ), patch.object( - service, - "_determine_document_type_based_on_common_name", - return_value=SnomedCodes.PATIENT_DATA.value, - ), patch.object( - service, - "is_uuid", - return_value=True, - ), patch.object( - service, - "delete_fhir_document_reference_by_nhs_id_and_doc_id", - return_value=MOCK_DOCUMENT_REFERENCE, - ) as mock_delete: - - result = service.process_fhir_document_reference(event) - - assert result == MOCK_DOCUMENT_REFERENCE - - mock_delete.assert_called_once_with( - nhs_number=deletion_identifiers[1], - doc_id=deletion_identifiers[0], - doc_type=SnomedCodes.PATIENT_DATA.value, - ) - - def test_process_returns_none_when_only_one_identifier(service): event = {"requestContext": {}} diff --git a/lambdas/tests/unit/services/test_pdm_document_service.py b/lambdas/tests/unit/services/test_pdm_document_service.py new file mode 100644 index 0000000000..6f843f059b --- /dev/null +++ b/lambdas/tests/unit/services/test_pdm_document_service.py @@ -0,0 +1,160 @@ +from unittest.mock import MagicMock + +import pytest + +from enums.snomed_codes import SnomedCodes +from models.document_reference import DocumentReference +from services.document_service import DocumentService + + +@pytest.fixture +def preliminary_dynamo_item_dict(): + return { + "Item": { + "id": "test-doc-id", + "nhs_number": "9000000001", + "s3_file_key": f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", + "s3_bucket_name": "test-staging-bucket", + "file_size": 1234567890, + "DocStatus": "preliminary", + "status": "current", + "file_name": None, + }, + } + + +@pytest.fixture +def service(): + service = DocumentService() + service.filter_item = MagicMock(return_value=True) + service.dynamo_service = MagicMock() + return service + + +@pytest.mark.parametrize( + "db_filters,expected_result", + [ + ( + [], + True, + ), + ( + [{"DocStatus": "preliminary"}], + True, + ), + ( + [{"DocStatus": "preliminary"}, {"status": "current"}], + True, + ), + ( + [{"DocStatus": "final"}], + False, + ), + ( + [{"DocStatus": "final"}, {"status": "foobar"}], + False, + ), + ( + [{"DocStatus": "preliminary"}, {"status": "foobar"}], + False, + ), + ], +) +def test_filter_returns_true(db_filters, expected_result, preliminary_dynamo_item_dict): + service = DocumentService() + result = service.filter_item(preliminary_dynamo_item_dict, filters=db_filters) + assert result is expected_result + + +def test_get_item_returns_none_when_no_item(service): + service.dynamo_service.get_item.return_value = {} + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "123"}, + model_class=DocumentReference, + ) + + assert result is None + service.dynamo_service.get_item.assert_called_once() + + +def test_get_item_returns_successful_document_reference( + service, + preliminary_dynamo_item_dict, +): + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + ) + + assert isinstance(result, DocumentReference) + assert result.id == "test-doc-id" + + +def test_get_item_does_not_return_successful_deleted_document_reference( + service, + preliminary_dynamo_item_dict, +): + preliminary_dynamo_item_dict["Item"]["Deleted"] = "foobar" + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + ) + + assert result is None + service.dynamo_service.get_item.assert_called_once() + + +def test_get_item_returns_successful_deleted_document_reference( + service, + preliminary_dynamo_item_dict, +): + preliminary_dynamo_item_dict["Item"]["Deleted"] = "foobar" + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + return_deleted=True, + ) + + assert isinstance(result, DocumentReference) + assert result.id == "test-doc-id" + + +def test_get_item_returns_none_when_filters_do_not_match( + service, + preliminary_dynamo_item_dict, +): + service.filter_item.return_value = False + service.dynamo_service.get_item.return_value = preliminary_dynamo_item_dict + + result = service._get_item( + table_name="dev_COREDocumentMetadata", + key={"ID": "test-doc-id"}, + model_class=DocumentReference, + ) + + assert result is None + service.dynamo_service.get_item.assert_called_once() + + +def test_get_item_validation_error_returns_none(service): + # invalid doc (id must be string) + service.dynamo_service.get_item.return_value = {"Item": {"ID": None}} + + result = service._get_item( + table_name="table", + key={"ID": "123"}, + model_class=DocumentReference, + ) + + assert result is None diff --git a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py index 4f051123c2..04d4cfe178 100644 --- a/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py +++ b/lambdas/tests/unit/services/test_pdm_get_fhir_document_reference_by_id_service.py @@ -1,4 +1,5 @@ import pytest + from enums.infrastructure import DynamoTables from enums.lambda_error import LambdaError from enums.snomed_codes import SnomedCode, SnomedCodes @@ -24,11 +25,10 @@ def patched_service(mocker, set_env, context): def test_get_document_reference_service(patched_service): documents = create_test_doc_store_refs() - patched_service.document_service.fetch_documents_from_table.return_value = documents + patched_service.document_service.get_item.return_value = documents[0] actual = patched_service.get_core_document_references( document_id="3d8683b9-1665-40d2-8499-6e8302d507ff", - snomed_code=SnomedCodes.PATIENT_DATA.value.code, table=MOCK_PDM_TABLE_NAME, ) assert actual == documents[0] @@ -40,11 +40,14 @@ def test_handle_get_document_reference_request(patched_service, mocker, set_env) expected = documents[0] mock_document_ref = documents[0] mocker.patch.object( - patched_service, "get_core_document_references", return_value=mock_document_ref + patched_service, + "get_core_document_references", + return_value=mock_document_ref, ) actual = patched_service.handle_get_document_reference_request( - SnomedCodes.PATIENT_DATA.value.code, "test-id" + SnomedCodes.PATIENT_DATA.value.code, + "test-id", ) assert expected == actual @@ -65,11 +68,11 @@ def test_get_dynamo_table_for_unsupported_doc_type(patched_service): non_lg_code = SnomedCode(code="non-lg-code", display_name="Non Lloyd George") - with pytest.raises(InvalidDocTypeException) as excinfo: + with pytest.raises(InvalidDocTypeException) as exc_info: patched_service._get_dynamo_table_for_doc_type(non_lg_code) - assert excinfo.value.status_code == 400 - assert excinfo.value.error == LambdaError.DocTypeDB + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.DocTypeDB # Not PDM however the code that this relates to was introduced because of PDM @@ -84,14 +87,12 @@ def test_get_dynamo_table_for_lloyd_george_doc_type(patched_service): def test_get_document_references_empty_result(patched_service): # Test when no documents are found - patched_service.document_service.fetch_documents_from_table.return_value = [] + patched_service.document_service.get_item.return_value = None with pytest.raises(GetFhirDocumentReferenceException) as exc_info: patched_service.get_core_document_references( document_id="test-id", - snomed_code=SnomedCodes.PATIENT_DATA.value.code, table=MOCK_PDM_TABLE_NAME, ) - - assert exc_info.value.error == LambdaError.DocumentReferenceNotFound assert exc_info.value.status_code == 404 + assert exc_info.value.error == LambdaError.DocumentReferenceNotFound diff --git a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py index 4bbc9c636f..9102f64ef5 100644 --- a/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py +++ b/lambdas/tests/unit/services/test_pdm_upload_document_reference_service.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest from botocore.exceptions import ClientError @@ -6,6 +6,7 @@ from enums.infrastructure import DynamoTables from enums.virus_scan_result import VirusScanResult from lambdas.enums.snomed_codes import SnomedCodes +from lambdas.services.document_service import DocumentService from models.document_reference import DocumentReference from services.mock_virus_scan_service import MockVirusScanService from services.upload_document_reference_service import UploadDocumentReferenceService @@ -14,8 +15,8 @@ MOCK_PDM_BUCKET, MOCK_PDM_TABLE_NAME, MOCK_STAGING_STORE_BUCKET, + WORKSPACE, ) -from utils.common_query_filters import PreliminaryStatus from utils.exceptions import DocumentServiceException, FileProcessingException from utils.lambda_exceptions import InvalidDocTypeException @@ -88,159 +89,152 @@ def mock_pdm_document_reference(): return doc_ref +@pytest.fixture +def dynamo_item_dict(): + return { + "Item": { + "id": "test-doc-id", + "nhs_number": "9000000001", + "s3_file_key": f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", + "s3_bucket_name": "test-staging-bucket", + "file_size": 1234567890, + "DocStatus": "preliminary", + "status": "current", + "file_name": None, + }, + } + + +@pytest.fixture +def pdm_document_reference(): + return DocumentReference( + id="test-doc-id", + nhs_number="9000000001", + s3_file_key=f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id", + s3_bucket_name="test-staging-bucket", + file_size=1234567890, + doc_status="preliminary", + status="current", + file_name=None, + author=None, + content_type="application/pdf", + created="2026-03-03T16:10:10.165878Z", + document_scan_creation="2026-03-03", + document_snomed_code_type="16521000000101", + file_location="s3://test-staging-bucket/9000000001/test-doc-id", + last_updated=1772554210, + raw_request=None, + s3_version_id=None, + s3_upload_key="9000000001/test-doc-id", + ttl=None, + version="1", + ) + + @pytest.fixture def pdm_service(set_env, mock_virus_scan_service): - with patch.multiple( - "services.upload_document_reference_service", - DocumentService=Mock(), - DynamoDBService=Mock(), - S3Service=Mock(), - ): - service = UploadDocumentReferenceService() - service.document_service = Mock() - service.dynamo_service = Mock() - service.virus_scan_service = MockVirusScanService() - service.s3_service = Mock() - service.table_name = MOCK_PDM_TABLE_NAME - service.destination_bucket_name = MOCK_PDM_BUCKET - service.doc_type = SnomedCodes.PATIENT_DATA.value - return service + service = UploadDocumentReferenceService() + service.dynamo_service = MagicMock() + service.virus_scan_service = MockVirusScanService() + service.s3_service = MagicMock() + service.table_name = "dev_COREDocumentMetadata" + service.destination_bucket_name = MOCK_PDM_BUCKET + service.doc_type = SnomedCodes.PATIENT_DATA.value + service.document_service = DocumentService() + service.document_service.dynamo_service = service.dynamo_service + return service def test_handle_upload_document_reference_request_with_empty_object_key(pdm_service): """Test handling of an empty object key""" - pdm_service.handle_upload_document_reference_request("", 122) - - pdm_service.document_service.fetch_documents_from_table.assert_not_called() + result = pdm_service.handle_upload_document_reference_request("", 122) + assert result is None def test_handle_upload_document_reference_request_with_none_object_key(pdm_service): """Test handling of a None object key""" - pdm_service.handle_upload_document_reference_request(None, 122) - - pdm_service.document_service.fetch_documents_from_table.assert_not_called() + result = pdm_service.handle_upload_document_reference_request(None, 122) + assert result is None def test_handle_upload_document_reference_request_success( - service, - mock_pdm_document_reference, + pdm_service, + dynamo_item_dict, mocker, ): """Test successful handling of the upload document reference request""" object_key = ( f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/9000000001/test-doc-id" ) - object_size = 1111 + object_size = dynamo_item_dict["Item"]["file_size"] - service.document_service.fetch_documents_from_table.side_effect = [ - [mock_pdm_document_reference], - ] - service.virus_scan_service.scan_file = mocker.MagicMock( + pdm_service.dynamo_service.get_item.return_value = { + "Item": dynamo_item_dict["Item"], + } + pdm_service.virus_scan_service.scan_file = mocker.MagicMock( return_value=VirusScanResult.CLEAN, ) - service.handle_upload_document_reference_request(object_key, object_size) - - service.document_service.fetch_documents_from_table.assert_called_once() - service.s3_service.copy_across_bucket.assert_called_once() - service.s3_service.delete_object.assert_called_once() - service.virus_scan_service.scan_file.assert_called_once() - - -def test_handle_upload_document_reference_request_with_exception(pdm_service): - """Test handling of exceptions during processing""" - object_key = "staging/test-doc-id" + pdm_service.handle_upload_document_reference_request(object_key, object_size) + pdm_service.s3_service.copy_across_bucket.assert_called_once() + pdm_service.s3_service.delete_object.assert_called_once() + pdm_service.virus_scan_service.scan_file.assert_called_once() - pdm_service.document_service.fetch_documents_from_table.side_effect = Exception( - "Test error", - ) - pdm_service.handle_upload_document_reference_request(object_key) +def test_handle_upload_document_reference_with_no_object_key(pdm_service): + object_key = "" + result = pdm_service.handle_upload_document_reference_request(object_key=object_key) + assert result is None def test_fetch_preliminary_document_reference_success( pdm_service, - mock_pdm_document_reference, + dynamo_item_dict, + pdm_document_reference, + mocker, ): """Test successful document reference fetching""" - document_key = "test-doc-id" - nhs_number = "12345" - pdm_service.document_service.fetch_documents_from_table.return_value = [ - mock_pdm_document_reference, - ] + spy = mocker.spy(pdm_service.document_service, "get_item") + document_key = dynamo_item_dict["Item"]["id"] + pdm_service.dynamo_service.get_item.return_value = { + "Item": dynamo_item_dict["Item"], + } result = pdm_service._fetch_preliminary_document_reference( document_key=document_key, - nhs_number=nhs_number, ) - - assert result == mock_pdm_document_reference - pdm_service.document_service.fetch_documents_from_table.assert_called_once_with( - table_name=MOCK_PDM_TABLE_NAME, - search_condition=[nhs_number, document_key], - search_key=["NhsNumber", "ID"], - query_filter=PreliminaryStatus, + assert isinstance(result, DocumentReference) + assert result.id == pdm_document_reference.id + spy.assert_called_once_with( + document_id=document_key, + table_name=f"dev_{MOCK_PDM_TABLE_NAME}", + return_deleted=False, + filters=[{"DocStatus": "preliminary"}], ) def test_fetch_preliminary_document_reference_no_documents_found(pdm_service): """Test handling when no documents are found""" document_key = "test-doc-id" - nhs_number = "12345" - pdm_service.document_service.fetch_documents_from_table.return_value = [] + pdm_service.dynamo_service.get_item.return_value = {} result = pdm_service._fetch_preliminary_document_reference( document_key=document_key, - nhs_number=nhs_number, ) assert result is None -def test_fetch_preliminary_document_reference_no_nhs_number(pdm_service): - """Test handling when no documents are found""" - document_key = "test-doc-id" - nhs_number = None - with pytest.raises(FileProcessingException): - pdm_service._fetch_preliminary_document_reference( - document_key=document_key, - nhs_number=nhs_number, - ) - - -def test_fetch_preliminary_document_reference_multiple_documents_warning( - pdm_service, - mock_document_reference, -): - """Test handling when multiple documents are found""" - document_key = "test-doc-id" - nhs_number = "12345" - mock_doc_2 = Mock(spec=DocumentReference) - pdm_service.document_service.fetch_documents_from_table.return_value = [ - mock_document_reference, - mock_doc_2, - ] - - result = pdm_service._fetch_preliminary_document_reference( - document_key=document_key, - nhs_number=nhs_number, - ) - - assert result == mock_document_reference - - -def test_fetch_preliminary_document_reference_exception(pdm_service): +def test_fetch_preliminary_document_reference_exception(pdm_service, dynamo_item_dict): """Test handling of exceptions during document fetching""" - document_key = "test-doc-id" - nhs_number = "12345" - pdm_service.document_service.fetch_documents_from_table.side_effect = ( + document_key = dynamo_item_dict["Item"]["id"] + pdm_service.dynamo_service.get_item.side_effect = ( ClientError({"error": "test error message"}, "test"), ) with pytest.raises(DocumentServiceException): pdm_service._fetch_preliminary_document_reference( document_key=document_key, - nhs_number=nhs_number, ) @@ -280,7 +274,7 @@ def test__process_preliminary_document_reference_clean_virus_scan( def test__process_preliminary_document_reference_infected_virus_scan( pdm_service, - mock_document_reference, + mock_pdm_document_reference, mocker, ): """Test processing document reference with an infected virus scan""" @@ -296,7 +290,7 @@ def test__process_preliminary_document_reference_infected_virus_scan( mock_process_clean = mocker.patch.object(pdm_service, "_process_clean_document") mock_update_dynamo = mocker.patch.object(pdm_service, "_update_dynamo_table") pdm_service._process_preliminary_document_reference( - mock_document_reference, + mock_pdm_document_reference, object_key, 1222, ) @@ -308,11 +302,11 @@ def test__process_preliminary_document_reference_infected_virus_scan( def test_perform_virus_scan_returns_clean_hardcoded( pdm_service, - mock_document_reference, + mock_pdm_document_reference, ): """Test virus scan returns hardcoded CLEAN result""" object_key = "staging/test-doc-id" - result = pdm_service._perform_virus_scan(mock_document_reference, object_key) + result = pdm_service._perform_virus_scan(mock_pdm_document_reference, object_key) assert result == VirusScanResult.CLEAN @@ -467,13 +461,14 @@ def test_delete_file_from_staging_bucket_client_error(pdm_service): def test_update_dynamo_table_clean_scan_result( pdm_service, mock_pdm_document_reference, + mocker, ): + spy = mocker.spy(pdm_service.document_service, "update_document") """Test updating DynamoDB table with a clean scan result""" pdm_service._update_dynamo_table(mock_pdm_document_reference) - - pdm_service.document_service.update_document.assert_called_once_with( - table_name=MOCK_PDM_TABLE_NAME, - key_pair={"NhsNumber": "9000000001", "ID": "test-doc-id"}, + spy.assert_called_once_with( + table_name=f"{WORKSPACE}_{MOCK_PDM_TABLE_NAME}", + key_pair={"ID": "test-doc-id"}, document=mock_pdm_document_reference, update_fields_name={ "virus_scanner_result", @@ -487,14 +482,19 @@ def test_update_dynamo_table_clean_scan_result( ) -def test_update_dynamo_table_infected_scan_result(pdm_service, mock_document_reference): +def test_update_dynamo_table_infected_scan_result( + pdm_service, + mock_document_reference, + mocker, +): """Test updating DynamoDB table with an infected scan result""" + spy = mocker.spy(pdm_service.document_service, "update_document") pdm_service._update_dynamo_table(mock_document_reference) - pdm_service.document_service.update_document.assert_called_once() + spy.assert_called_once() -def test_update_dynamo_table_client_error(pdm_service, mock_document_reference): +def test_update_dynamo_table_client_error(pdm_service, pdm_document_reference, mocker): """Test handling of ClientError during DynamoDB update""" client_error = ClientError( error_response={ @@ -505,10 +505,11 @@ def test_update_dynamo_table_client_error(pdm_service, mock_document_reference): }, operation_name="UpdateItem", ) + pdm_service.document_service = MagicMock() pdm_service.document_service.update_document.side_effect = client_error with pytest.raises(DocumentServiceException): - pdm_service._update_dynamo_table(mock_document_reference) + pdm_service._update_dynamo_table(pdm_document_reference) def test_handle_upload_document_reference_request_no_document_found(pdm_service): @@ -516,14 +517,14 @@ def test_handle_upload_document_reference_request_no_document_found(pdm_service) object_key = "staging/test-doc-id" object_size = 1234 - pdm_service.document_service.fetch_documents_from_table.return_value = [] + pdm_service.dynamo_service.get_item.return_value = {} - pdm_service.handle_upload_document_reference_request(object_key, object_size) + result = pdm_service.handle_upload_document_reference_request( + object_key, + object_size, + ) - # Should fetch but not proceed with processing - pdm_service.document_service.fetch_documents_from_table.assert_called_once() - pdm_service.s3_service.copy_across_bucket.assert_not_called() - pdm_service.document_service.update_document.assert_not_called() + assert result is None def test_process_preliminary_document_reference_exception_during_processing( @@ -654,44 +655,19 @@ def test_document_type_extraction_from_object_key( assert service.doc_type.code == expected_doctype.code -def test_handle_upload_pdm_document_reference_request_success( - service, - mock_document_reference, - mocker, -): - """Test successful handling of the upload document reference request""" - pdm_snomed = SnomedCodes.PATIENT_DATA.value - object_key = f"fhir_upload/{pdm_snomed.code}/staging/test-doc-id" - object_size = 1111 - service.document_service.fetch_documents_from_table.return_value = [ - mock_document_reference, - ] - service.virus_scan_service.scan_file = mocker.MagicMock( - return_value=VirusScanResult.CLEAN, - ) - - service.handle_upload_document_reference_request(object_key, object_size) - - service.document_service.fetch_documents_from_table.assert_called_once() - service.document_service.update_document.assert_called_once() - service.s3_service.copy_across_bucket.assert_called_once() - service.s3_service.delete_object.assert_called_once() - service.virus_scan_service.scan_file.assert_called_once() - - def test_copy_files_from_staging_bucket_to_pdm_success( pdm_service, - mock_pdm_document_reference, + dynamo_item_dict, + pdm_document_reference, + mocker, ): """Test successful file copying from staging bucket""" - source_file_key = ( - f"fhir_upload/{SnomedCodes.PATIENT_DATA.value.code}/staging/test-doc-id" - ) + source_file_key = dynamo_item_dict["Item"]["s3_file_key"] expected_dest_key = ( - f"{mock_pdm_document_reference.nhs_number}/{mock_pdm_document_reference.id}" + f"{pdm_document_reference.nhs_number}/{pdm_document_reference.id}" ) pdm_service.copy_files_from_staging_bucket( - mock_pdm_document_reference, + pdm_document_reference, source_file_key, ) pdm_service.s3_service.copy_across_bucket.assert_called_once_with( @@ -701,5 +677,5 @@ def test_copy_files_from_staging_bucket_to_pdm_success( dest_file_key=expected_dest_key, ) - assert mock_pdm_document_reference.s3_file_key == expected_dest_key - assert mock_pdm_document_reference.s3_bucket_name == MOCK_PDM_BUCKET + assert pdm_document_reference.s3_file_key == expected_dest_key + assert pdm_document_reference.s3_bucket_name == MOCK_PDM_BUCKET