From 438ff3c77d01bd530567906f3f1cb62d3e59495f Mon Sep 17 00:00:00 2001 From: Natasa Fragkou Date: Wed, 12 Jun 2024 09:52:33 +0100 Subject: [PATCH] Update: [AEA-4021] - Reject duplicate requests (#287) ## Summary Dispensing systems will send updatePrescriptionStatus requests to the PSU API to provide status updates on prescription items which are being processed by pharmacies using the system. The status updates will include a last updated time stamp and the NPPT service is only interested in the tracking status with the most recent time stamp. Although all status updates will be recorded in the data store, duplicate updates must be rejected. https://nhsd-jira.digital.nhs.uk/browse/AEA-4021 - :sparkles: New Feature --- package-lock.json | 7 +- .../src/updatePrescriptionStatus.ts | 49 +++++++++---- .../src/utils/databaseClient.ts | 17 ++++- .../src/utils/responses.ts | 33 +++++++++ .../tests/testDatabaseClient.test.ts | 68 +++++++++++++++++++ .../tests/testHandler.test.ts | 18 ++++- .../testUpdatePrescriptionStatus.test.ts | 39 ++++++++++- .../tests/utils/testUtils.ts | 2 + 8 files changed, 211 insertions(+), 22 deletions(-) create mode 100644 packages/updatePrescriptionStatus/tests/testDatabaseClient.test.ts diff --git a/package-lock.json b/package-lock.json index 1db5af9de..f5650e8f2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4413,6 +4413,7 @@ "version": "6.12.6", "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", + "dev": true, "dependencies": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", @@ -6668,7 +6669,8 @@ "node_modules/fast-json-stable-stringify": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz", - "integrity": "sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==" + "integrity": "sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==", + "dev": true }, "node_modules/fast-levenshtein": { "version": "2.0.6", @@ -8695,7 +8697,8 @@ "node_modules/json-schema-traverse": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz", - "integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==" + "integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==", + "dev": true }, "node_modules/json-stable-stringify-without-jsonify": { "version": "1.0.1", diff --git a/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts b/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts index 2232745ba..0c5b65a2c 100644 --- a/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts +++ b/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts @@ -14,10 +14,12 @@ import { accepted, badRequest, bundleWrap, + conflictDuplicate, createSuccessResponseEntries, serverError, timeoutResponse } from "./utils/responses" +import {TransactionCanceledException} from "@aws-sdk/client-dynamodb" const LAMBDA_TIMEOUT_MS = 9500 const logger = new Logger({serviceName: "updatePrescriptionStatus"}) @@ -73,22 +75,31 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise, responseEntr return valid } +export function handleTransactionCancelledException( + e: TransactionCanceledException, + responseEntries: Array +): void { + e.CancellationReasons?.forEach((reason) => { + if (reason.Item?.TaskID?.S) { + const taskId = reason.Item.TaskID.S + responseEntries.push(conflictDuplicate(taskId)) + } + }) +} + export function buildDataItems( requestEntries: Array, xRequestID: string, diff --git a/packages/updatePrescriptionStatus/src/utils/databaseClient.ts b/packages/updatePrescriptionStatus/src/utils/databaseClient.ts index d7d7c2a20..f1800fdfe 100644 --- a/packages/updatePrescriptionStatus/src/utils/databaseClient.ts +++ b/packages/updatePrescriptionStatus/src/utils/databaseClient.ts @@ -1,11 +1,16 @@ import {Logger} from "@aws-lambda-powertools/logger" -import {DynamoDBClient, TransactWriteItem, TransactWriteItemsCommand} from "@aws-sdk/client-dynamodb" +import { + DynamoDBClient, + TransactWriteItem, + TransactWriteItemsCommand, + TransactionCanceledException +} from "@aws-sdk/client-dynamodb" import {marshall} from "@aws-sdk/util-dynamodb" import {DataItem} from "../updatePrescriptionStatus" import {Timeout} from "./timeoutUtils" -const logger = new Logger({serviceName: "databaseClient"}) +export const logger = new Logger({serviceName: "databaseClient"}) const client = new DynamoDBClient() const tableName = process.env.TABLE_NAME ?? "PrescriptionStatusUpdates" @@ -15,7 +20,9 @@ function createTransactionCommand(dataItems: Array): TransactWriteItem return { Put: { TableName: tableName, - Item: marshall(d) + Item: marshall(d), + ConditionExpression: "attribute_not_exists(TaskID) AND attribute_not_exists(PrescriptionID)", + ReturnValuesOnConditionCheckFailure: "ALL_OLD" } } }) @@ -30,6 +37,10 @@ export async function persistDataItems(dataItems: Array): Promise) { return entries.map((e) => created(e.fullUrl!)) } diff --git a/packages/updatePrescriptionStatus/tests/testDatabaseClient.test.ts b/packages/updatePrescriptionStatus/tests/testDatabaseClient.test.ts new file mode 100644 index 000000000..603cecc47 --- /dev/null +++ b/packages/updatePrescriptionStatus/tests/testDatabaseClient.test.ts @@ -0,0 +1,68 @@ +import { + expect, + describe, + it, + jest +} from "@jest/globals" + +import {TransactionCanceledException} from "@aws-sdk/client-dynamodb" +import {mockDynamoDBClient} from "./utils/testUtils" + +const {mockSend} = mockDynamoDBClient() +const {persistDataItems, logger} = await import("../src/utils/databaseClient") + +describe("Unit test persistDataItems", () => { + beforeEach(() => { + jest.resetModules() + jest.clearAllMocks() + }) + it("when the conditional check fails, an error is thrown", async () => { + const dataItems = [ + { + LastModified: "2023-01-01T00:00:00Z", + LineItemID: "LineItemID_1", + PatientNHSNumber: "PatientNHSNumber_1", + PharmacyODSCode: "PharmacyODSCode_1", + PrescriptionID: "PrescriptionID_1", + RequestID: "RequestID_1", + Status: "Status_1", + TaskID: "TaskID_1", + TerminalStatus: "TerminalStatus_1", + ApplicationName: "name" + }, + { + LastModified: "2023-01-02T00:00:00Z", + LineItemID: "LineItemID_2", + PatientNHSNumber: "PatientNHSNumber_2", + PharmacyODSCode: "PharmacyODSCode_2", + PrescriptionID: "PrescriptionID_1", + RequestID: "RequestID_2", + Status: "Status_2", + TaskID: "TaskID_1", + TerminalStatus: "TerminalStatus_2", + ApplicationName: "name" + } + ] + + mockSend.mockRejectedValue( + new TransactionCanceledException({ + $metadata: {}, + message: "DynamoDB transaction cancelled due to conditional check failure.", + CancellationReasons: [{Code: "ConditionalCheckFailedException"}] + }) as never + ) + const loggerSpy = jest.spyOn(logger, "error") + + await expect(persistDataItems(dataItems)).rejects.toThrow( + new TransactionCanceledException({ + $metadata: {}, + message: "DynamoDB transaction cancelled due to conditional check failure.", + CancellationReasons: [{Code: "ConditionalCheckFailedException"}] + }) as never + ) + + expect(loggerSpy).toHaveBeenCalledWith("DynamoDB transaction cancelled due to conditional check failure.", { + reasons: [{Code: "ConditionalCheckFailedException"}] + }) + }) +}) diff --git a/packages/updatePrescriptionStatus/tests/testHandler.test.ts b/packages/updatePrescriptionStatus/tests/testHandler.test.ts index 4afcbc31f..370588ecd 100644 --- a/packages/updatePrescriptionStatus/tests/testHandler.test.ts +++ b/packages/updatePrescriptionStatus/tests/testHandler.test.ts @@ -30,6 +30,7 @@ import { serverError, timeoutResponse } from "../src/utils/responses" +import {TransactionCanceledException} from "@aws-sdk/client-dynamodb" const {mockSend, mockTransact} = mockDynamoDBClient() const {handler} = await import("../src/updatePrescriptionStatus") @@ -185,7 +186,6 @@ describe("Integration tests for updatePrescriptionStatus handler", () => { }) it("when x-request-id header is mixed case, expect it to work", async () => { - const body = generateBody() const event: APIGatewayProxyEvent = generateMockEvent(body) delete event.headers["x-request-id"] @@ -197,6 +197,22 @@ describe("Integration tests for updatePrescriptionStatus handler", () => { const response: APIGatewayProxyResult = await handler(event, {}) expect(response.statusCode).toEqual(201) + }) + + it("when duplicates are introduced, expect 409 status code and message indicating duplicate fields", async () => { + const body = generateBody() + const mockEvent: APIGatewayProxyEvent = generateMockEvent(body) + + mockSend.mockRejectedValue( + new TransactionCanceledException({ + $metadata: {}, + message: "DynamoDB transaction cancelled due to conditional check failure.", + CancellationReasons: [{Code: "ConditionalCheckFailedException"}] + }) as never + ) + + const result: APIGatewayProxyResult = await handler(mockEvent, {}) + expect(result.statusCode).toBe(409) }) }) diff --git a/packages/updatePrescriptionStatus/tests/testUpdatePrescriptionStatus.test.ts b/packages/updatePrescriptionStatus/tests/testUpdatePrescriptionStatus.test.ts index 4b1f20b29..8277808c9 100644 --- a/packages/updatePrescriptionStatus/tests/testUpdatePrescriptionStatus.test.ts +++ b/packages/updatePrescriptionStatus/tests/testUpdatePrescriptionStatus.test.ts @@ -8,13 +8,16 @@ import { import {BundleEntry} from "fhir/r4" -import {badRequest} from "../src/utils/responses" +import {badRequest, conflictDuplicate} from "../src/utils/responses" import {DEFAULT_DATE, X_REQUEST_ID, mockInternalDependency} from "./utils/testUtils" import {APIGatewayProxyEvent} from "aws-lambda" import * as content from "../src/validation/content" +import {TransactionCanceledException} from "@aws-sdk/client-dynamodb" const mockValidateEntry = mockInternalDependency("../src/validation/content", content, "validateEntry") -const {castEventBody, getXRequestID, validateEntries} = await import("../src/updatePrescriptionStatus") +const {castEventBody, getXRequestID, validateEntries, handleTransactionCancelledException} = await import( + "../src/updatePrescriptionStatus" +) describe("Unit test getXRequestID", () => { beforeAll(() => { @@ -64,7 +67,9 @@ describe("Unit test castEventBody", () => { expect(result).toEqual(undefined) expect(responseEntries.length).toEqual(1) - expect(responseEntries[0]).toEqual(badRequest("Request body does not have resourceType of 'Bundle' and type of 'transaction'.")) + expect(responseEntries[0]).toEqual( + badRequest("Request body does not have resourceType of 'Bundle' and type of 'transaction'.") + ) }) it("when body has correct resourceType and type, return bundle and no response entries", async () => { @@ -123,3 +128,31 @@ describe("Unit test validateEntries", () => { expect(inValidResponseEntry.response?.status).toEqual("400 Bad Request") }) }) + +describe("handleTransactionCancelledException", () => { + it("should add conflictDuplicate entries to responseEntries", () => { + const responseEntries: Array = [] + const mockException: TransactionCanceledException = { + name: "TransactionCanceledException", + message: "DynamoDB transaction cancelled due to conditional check failure.", + $fault: "client", + $metadata: {}, + CancellationReasons: [ + { + Code: "ConditionalCheckFailed", + Item: { + TaskID: {S: "d70678c-81e4-6665-8c67-17596fd0aa46"} + }, + Message: "The conditional request failed" + } + ] + } + + handleTransactionCancelledException(mockException, responseEntries) + const validResponseEntry = responseEntries[0] + + expect(responseEntries).toHaveLength(1) + expect(validResponseEntry).toEqual(conflictDuplicate("d70678c-81e4-6665-8c67-17596fd0aa46")) + expect(validResponseEntry.response?.status).toEqual("409 Conflict") + }) +}) diff --git a/packages/updatePrescriptionStatus/tests/utils/testUtils.ts b/packages/updatePrescriptionStatus/tests/utils/testUtils.ts index 544e1a733..08175a456 100644 --- a/packages/updatePrescriptionStatus/tests/utils/testUtils.ts +++ b/packages/updatePrescriptionStatus/tests/utils/testUtils.ts @@ -2,6 +2,7 @@ import {APIGatewayProxyEvent} from "aws-lambda" import {jest} from "@jest/globals" +import * as dynamo from "@aws-sdk/client-dynamodb" import { LINE_ITEM_ID_CODESYSTEM, @@ -146,6 +147,7 @@ export function mockDynamoDBClient() { const mockTransact = jest.fn() jest.unstable_mockModule("@aws-sdk/client-dynamodb", () => { return { + ...dynamo, DynamoDBClient: jest.fn().mockImplementation(() => ({ send: mockSend })),