Tests seembroken, but wrote an sqs client

Author: wildjames

package-lock.json

@@ -3,7 +3,7 @@ export const mockEventBridgeEvent = {
     version: "0",
     account: "123456789012",
     time: new Date().toISOString(),
-    region: "us-east-1",
+    region: "eu-west-2",
     resources: [],
     source: "aws.events",
     "detail-type": "Scheduled Event",

packages/common/testing/src/mockEventBridgeEvent.js

@@ -0,0 +1,4 @@
+/* eslint-disable no-undef */
+process.env.TABLE_NAME = "dummy_table";
+process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL = "dummy_notify_sqs";
+process.env.AWS_REGION = "eu-west-2";

packages/updatePrescriptionStatus/.jest/setEnvVars.js

@@ -3,7 +3,8 @@ import defaultConfig from "../../jest.default.config"
 
 const jestConfig: JestConfigWithTsJest = {
   ...defaultConfig,
-  "rootDir": "./"
+  "rootDir": "./",
+  setupFiles: ["<rootDir>/.jest/setEnvVars.js"]
 }
 
 export default jestConfig

packages/updatePrescriptionStatus/jest.config.ts

@@ -17,6 +17,7 @@
     "@aws-lambda-powertools/commons": "^2.17.0",
     "@aws-lambda-powertools/logger": "^2.18.0",
     "@aws-sdk/client-dynamodb": "^3.772.0",
+    "@aws-sdk/client-sqs": "^3.787.0",
     "@aws-sdk/util-dynamodb": "^3.787.0",
     "@middy/core": "^6.1.6",
     "@middy/http-header-normalizer": "^6.1.6",

packages/updatePrescriptionStatus/package.json

@@ -165,7 +165,12 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
 
   // This prescription was handled successfully,
   // so add a message to the notifications SQS
-  pushPrescriptionToNotificationSQS(dataItems)
+  try {
+    await pushPrescriptionToNotificationSQS(dataItems, logger)
+  } catch (err) {
+    logger.error("Failed to push prescriptions to the notifications SQS", {err})
+    // DO NOT throw an error here, since we want to still return the update!
+  }
 
   return response(201, responseEntries)
 }

packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts

@@ -1,8 +1,62 @@
+import {SQSClient, SendMessageBatchCommand} from "@aws-sdk/client-sqs"
+import {v4} from "uuid"
+
 import {Logger} from "@aws-lambda-powertools/logger"
 import {DataItem} from "../updatePrescriptionStatus"
 
 const sqsUrl = process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL
 
-export function pushPrescriptionToNotificationSQS(data: Array<DataItem>, logger: Logger) {
+// The AWS_REGION is always defined in lambda environments
+const sqs = new SQSClient({region: process.env.AWS_REGION})
+
+// Returns the original array, chunked in batches of up to <size>
+function chunkArray<T>(arr: Array<T>, size: number): Array<Array<T>> {
+  const chunks: Array<Array<T>> = []
+  for (let i = 0; i < arr.length; i += size) {
+    chunks.push(arr.slice(i, i + size))
+  }
+  return chunks
+}
+
+/**
+ * Pushes an array of DataItems to the notifications SQS queue
+ * Uses SendMessageBatch to send up to 10 at a time
+ *
+ * @param data - Array of DataItems to send to SQS
+ * @param logger - Logger instance
+ */
+export async function pushPrescriptionToNotificationSQS(data: Array<DataItem>, logger: Logger) {
   logger.info("Pushing data items up to the notifications SQS", {data, sqsUrl})
+
+  if (!sqsUrl) {
+    logger.error("Notifications SQS URL not found in environment variables")
+    throw new Error("Notifications SQS URL not configured")
+  }
+
+  // SQS batch calls are limited to 10 messages per request, so chunk the data
+  const batches = chunkArray(data, 10)
+
+  for (const batch of batches) {
+    // Create SQS entries. Each message is required to have an unique Id string.
+    // TODO: I'm creating a new UUID here, but am I safe to use the lineID? It looks like it should be unique
+    const entries = batch.map((item) => ({
+      Id: v4().toUpperCase(),
+      // Id: item.LineItemID || v4().toUpperCase(),
+      MessageBody: JSON.stringify(item)
+    }))
+
+    const params = {
+      QueueUrl: sqsUrl,
+      Entries: entries
+    }
+
+    try {
+      const command = new SendMessageBatchCommand(params)
+      const result = await sqs.send(command)
+      logger.info("Successfully sent a batch of prescriptions to the notifications SQS", {result})
+    } catch (error) {
+      logger.error("Failed to send a batch of prescriptions to the notifications SQS", {error})
+      throw error
+    }
+  }
 }

packages/updatePrescriptionStatus/src/utils/sqsClient.ts

@@ -17,6 +17,7 @@ import {
   generateExpectedItems,
   generateMockEvent,
   mockDynamoDBClient,
+  mockSQSClient,
   TASK_VALUES
 } from "./utils/testUtils"
 
@@ -36,7 +37,8 @@ import {
 } from "../src/utils/responses"
 import {QueryCommand, TransactionCanceledException, TransactWriteItemsCommand} from "@aws-sdk/client-dynamodb"
 
-const {mockSend} = mockDynamoDBClient()
+const {mockSend: dynamoDBMockSend} = mockDynamoDBClient()
+const {mockSend: sqsMockSend} = mockSQSClient()
 const {handler, logger} = await import("../src/updatePrescriptionStatus")
 const LAMBDA_TIMEOUT_MS = 9500 // 9.5 sec
 
@@ -75,7 +77,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
     expect(response.statusCode).toEqual(201)
     expect(JSON.parse(response.body)).toEqual(responseSingleItem)
 
-    expect(mockSend).toHaveBeenCalledWith(
+    expect(dynamoDBMockSend).toHaveBeenCalledWith(
       expect.objectContaining(expectedItems)
     )
   })
@@ -101,7 +103,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
     expect(JSON.parse(response.body)).toEqual(responseSingleItem)
 
     expect(expectedItems.input.TransactItems[0].Put.Item.RepeatNo).toEqual(undefined)
-    expect(mockSend).toHaveBeenCalledWith(
+    expect(dynamoDBMockSend).toHaveBeenCalledWith(
       expect.objectContaining(expectedItems)
     )
   })
@@ -126,7 +128,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
     expect(JSON.parse(response.body)).toEqual(responseSingleItem)
 
     expect(expectedItems.input.TransactItems[0].Put.Item.RepeatNo).toEqual(1)
-    expect(mockSend).toHaveBeenCalledWith(
+    expect(dynamoDBMockSend).toHaveBeenCalledWith(
       expect.objectContaining(expectedItems)
     )
   })
@@ -141,7 +143,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
     expect(response.statusCode).toEqual(201)
     expect(JSON.parse(response.body)).toEqual(responseMultipleItems)
 
-    expect(mockSend).toHaveBeenCalledWith(
+    expect(dynamoDBMockSend).toHaveBeenCalledWith(
       expect.objectContaining(expectedItems)
     )
   })
@@ -194,7 +196,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
 
   it("when dynamo call fails, expect 500 status code and internal server error message", async () => {
     const event = generateMockEvent(requestDispatched)
-    mockSend.mockRejectedValue(new Error() as never)
+    dynamoDBMockSend.mockRejectedValue(new Error() as never)
 
     const response: APIGatewayProxyResult = await handler(event, {})
 
@@ -203,7 +205,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
   })
 
   it("when data store update times out, expect 504 status code and relevant error message", async () => {
-    mockSend.mockImplementation((command) => new Promise((resolve) => {
+    dynamoDBMockSend.mockImplementation((command) => new Promise((resolve) => {
       if (!(command instanceof TransactWriteItemsCommand)) {
         resolve(false)
       }
@@ -279,7 +281,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
     const body = generateBody()
     const mockEvent: APIGatewayProxyEvent = generateMockEvent(body)
 
-    mockSend.mockRejectedValue(
+    dynamoDBMockSend.mockRejectedValue(
       new TransactionCanceledException({
         message:
           "DynamoDB transaction cancelled due to conditional check failure.",
@@ -324,7 +326,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
       requestDuplicateItems
     )
 
-    mockSend.mockRejectedValue(
+    dynamoDBMockSend.mockRejectedValue(
       new TransactionCanceledException({
         message:
           "DynamoDB transaction cancelled due to conditional check failure.",
@@ -375,7 +377,7 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
     const mockEvent: APIGatewayProxyEvent = generateMockEvent(body)
     const loggerSpy = jest.spyOn(logger, "info")
 
-    mockSend.mockImplementation(
+    dynamoDBMockSend.mockImplementation(
       async (command) => {
         if (command instanceof QueryCommand) {
           return new Object({Items: [
@@ -407,4 +409,17 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
       }
     )
   })
+
+  it("when the notification SQS push fails, the response still succeeds", async () => {
+    // TODO: I'm not convinced this is working...
+    sqsMockSend.mockImplementation(
+      async () => {
+        throw new Error("Test error")
+      }
+    )
+
+    const event: APIGatewayProxyEvent = generateMockEvent(requestDispatched)
+    const response: APIGatewayProxyResult = await handler(event, {})
+    expect(response.statusCode).toBe(201)
+  })
 })

packages/updatePrescriptionStatus/tests/testHandler.test.ts

@@ -3,6 +3,7 @@
 import {APIGatewayProxyEvent} from "aws-lambda"
 import {jest} from "@jest/globals"
 import * as dynamo from "@aws-sdk/client-dynamodb"
+import * as sqs from "@aws-sdk/client-sqs"
 
 import {
   LINE_ITEM_ID_CODESYSTEM,
@@ -180,3 +181,17 @@ export function mockDynamoDBClient() {
   })
   return {mockSend}
 }
+
+// Similarly mock the SQS client
+export function mockSQSClient() {
+  const mockSend = jest.fn()
+  jest.unstable_mockModule("@aws-sdk/client-sqs", () => {
+    return {
+      ...sqs,
+      SQSClient: jest.fn().mockImplementation(() => ({
+        send: mockSend
+      }))
+    }
+  })
+  return {mockSend}
+}