Fix: [AEA-0000] - Reduce the number of calls to the parameter store (#1844)

## Summary

- Routine Change

### Details

The parameter store was getting hit once per call for the PSU and notify
processor. Update the logic to cache values, and only fetch them once
per minute.

Author: wildjames

SAMtemplates/functions/lambda_resources.yaml

@@ -116,6 +116,7 @@ Resources:
           - Effect: Allow
             Action:
               - ssm:GetParameter
+              - ssm:GetParameters
             Resource:
               - "*"
 

packages/nhsNotifyLambda/src/utils.ts

@@ -8,7 +8,7 @@ import {
 } from "@aws-sdk/client-sqs"
 import {DynamoDBClient} from "@aws-sdk/client-dynamodb"
 import {DynamoDBDocumentClient, GetCommand, PutCommand} from "@aws-sdk/lib-dynamodb"
-import {getParameter} from "@aws-lambda-powertools/parameters/ssm"
+import {SSMProvider} from "@aws-lambda-powertools/parameters/ssm"
 import {getSecret} from "@aws-lambda-powertools/parameters/secrets"
 
 import {NotifyDataItem} from "@PrescriptionStatusUpdate_common/commonTypes"
@@ -30,7 +30,6 @@ const DUMMY_NOTIFY_DELAY_MS = 150
 // these are only ever changed by a deployment
 const dynamoTable = process.env.TABLE_NAME
 const sqsUrl = process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL
-const MAKE_REAL_NOTIFY_REQUESTS_PARAM = process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM
 
 // AWS clients
 const sqs = new SQSClient({region: process.env.AWS_REGION})
@@ -44,6 +43,25 @@ const marshallOptions = {
 const dynamo = new DynamoDBClient({region: process.env.AWS_REGION})
 const docClient = DynamoDBDocumentClient.from(dynamo, {marshallOptions})
 
+const ssm = new SSMProvider()
+const paramNames = {
+  [process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM!]: {maxAge: 60},
+  [process.env.NOTIFY_API_BASE_URL_PARAM!]: {maxAge: 60}
+}
+const configPromise = ssm.getParametersByName(paramNames)
+
+async function loadConfig(): Promise<{
+  makeRealNotifyRequests: boolean,
+  notifyApiBaseUrlRaw: string
+}> {
+  const all = await configPromise
+
+  return {
+    makeRealNotifyRequests: all[process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM!] === "true",
+    notifyApiBaseUrlRaw: all[process.env.NOTIFY_API_BASE_URL_PARAM!] as string
+  }
+}
+
 /**
  * Returns the original array, chunked in batches of up to <size>
  *
@@ -94,7 +112,7 @@ export async function reportQueueStatus(logger: Logger): Promise<void> {
 // and helps track the nhs notify results
 export interface NotifyDataItemMessage extends Message {
   PSUDataItem: NotifyDataItem
-  success?: boolean
+  deliveryStatus?: string
   messageBatchReference?: string,
   // message reference is our internal UUID for the message
   messageReference: string
@@ -309,7 +327,7 @@ export async function addPrescriptionMessagesToNotificationStateStore(
       RequestId: data.PSUDataItem.RequestID,
       SQSMessageID: data.MessageId,
       LastNotifiedPrescriptionStatus: data.PSUDataItem.Status,
-      DeliveryStatus: data.success ? "requested" : "notify request failed",
+      DeliveryStatus: data.deliveryStatus ?? "unknown", // Fall back to unknown if not set
       NotifyMessageID: data.notifyMessageId, // This is a GSI, but leaving it blank is fine
       NotifyMessageReference: data.messageReference,
       NotifyMessageBatchReference: data.messageBatchReference, // Will be undefined when request fails
@@ -413,12 +431,11 @@ export async function makeBatchNotifyRequest(
   routingPlanId: string,
   data: Array<NotifyDataItemMessage>
 ): Promise<Array<NotifyDataItemMessage>> {
-  // Fetch secrets and parameters
-  if (!process.env.NOTIFY_API_BASE_URL_PARAM || !process.env.API_KEY_SECRET) {
+  if (!process.env.API_KEY_SECRET) {
     throw new Error("Environment configuration error")
   }
 
-  const notifyApiBaseUrlRaw = await getParameter(process.env.NOTIFY_API_BASE_URL_PARAM)
+  const {makeRealNotifyRequests, notifyApiBaseUrlRaw} = await loadConfig()
   const apiKeyRaw = await getSecret(process.env.API_KEY_SECRET)
 
   if (!notifyApiBaseUrlRaw) throw new Error("NOTIFY_API_BASE_URL is not defined in the environment variables!")
@@ -479,6 +496,21 @@ export async function makeBatchNotifyRequest(
     return [...res1, ...res2]
   }
 
+  if (!makeRealNotifyRequests) {
+    logger.info("Not doing real Notify requests. Simply waiting for some time and returning success on all messages")
+    await new Promise(f => setTimeout(f, DUMMY_NOTIFY_DELAY_MS))
+
+    // Map each input item to a "successful" NotifyDataItemMessage
+    return data.map(item => {
+      return {
+        ...item,
+        messageBatchReference,
+        deliveryStatus: "silent running",
+        notifyMessageId: v4() // Create a dummy UUID
+      }
+    })
+  }
+
   logger.info("Making a request for notifications to NHS notify", {count: data.length, routingPlanId})
 
   // Create an axios instance configured for Notify
@@ -502,24 +534,6 @@ export async function makeBatchNotifyRequest(
     onRetry: onAxiosRetry
   })
 
-  let doRealRequest: boolean = false
-  if (MAKE_REAL_NOTIFY_REQUESTS_PARAM) doRealRequest = await getParameter(MAKE_REAL_NOTIFY_REQUESTS_PARAM) === "true"
-
-  if (!doRealRequest) {
-    logger.info("Not doing real Notify requests. Simply waiting for some time and returning success on all messages")
-    await new Promise(f => setTimeout(f, DUMMY_NOTIFY_DELAY_MS))
-
-    // Map each input item to a "successful" NotifyDataItemMessage
-    return data.map(item => {
-      return {
-        ...item,
-        messageBatchReference,
-        success: true,
-        notifyMessageId: v4() // Create a dummy UUID
-      }
-    })
-  }
-
   try {
     const resp = await axiosInstance.post<CreateMessageBatchResponse>("/v1/message-batches", body)
 
@@ -528,7 +542,7 @@ export async function makeBatchNotifyRequest(
       logger.info("Requested notifications OK!", {
         messageBatchReference,
         messageReferences: messages.map(e => e.messageReference),
-        success: "Requested Success"
+        deliveryStatus: "requested"
       })
 
       // Map each input item to a NotifyDataItemMessage, marking success and attaching the notify ID
@@ -541,7 +555,7 @@ export async function makeBatchNotifyRequest(
         return {
           ...item,
           messageBatchReference,
-          success: !!match,
+          deliveryStatus: match ? "requested" : "notify request failed",
           notifyMessageId: match?.id
         }
       })
@@ -552,7 +566,7 @@ export async function makeBatchNotifyRequest(
         statusText: resp.statusText,
         messageBatchReference,
         messageReferences: messages.map(e => e.messageReference),
-        success: "Requested Failed"
+        deliveryStatus: "notify request failed"
       })
       throw new Error("Notify batch request failed")
     }
@@ -561,7 +575,7 @@ export async function makeBatchNotifyRequest(
     logger.error("Notify batch request failed", {error})
     return data.map(item => ({
       ...item,
-      success: false,
+      deliveryStatus: "notify request failed",
       messageBatchReference,
       notifyMessageId: undefined
     }))

packages/nhsNotifyLambda/tests/testUtils.test.ts

@@ -11,17 +11,20 @@ import {constructMessage, constructPSUDataItemMessage, mockSQSClient} from "./te
 const {mockSend: sqsMockSend} = mockSQSClient()
 
 const TEST_URL = "https://example.com"
-const mockGetParameter = jest.fn().mockImplementation((name) => {
-  if (name === "NOTIFY_API_BASE_URL_PARAM") {
-    return TEST_URL
+const mockGetParametersByName = jest.fn(async () => Promise.resolve(
+  {
+    [process.env.NOTIFY_API_BASE_URL_PARAM!]: TEST_URL,
+    [process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM!]: "true"
   }
-  return name
-})
+))
+
 jest.unstable_mockModule(
   "@aws-lambda-powertools/parameters/ssm",
   async () => ({
     __esModule: true,
-    getParameter: mockGetParameter
+    SSMProvider: jest.fn().mockImplementation(() => ({
+      getParametersByName: mockGetParametersByName
+    }))
   })
 )
 
@@ -515,14 +518,14 @@ describe("NHS notify lambda helper functions", () => {
       expect(result).toHaveLength(2)
       expect(result[0]).toMatchObject({
         PSUDataItem: data[0].PSUDataItem,
-        success: true,
+        deliveryStatus: "requested",
         notifyMessageId: "msg-id-1",
         messageBatchReference: expect.any(String),
         messageReference: expect.any(String)
       })
       expect(result[1]).toMatchObject({
         PSUDataItem: data[1].PSUDataItem,
-        success: false,
+        deliveryStatus: "notify request failed",
         notifyMessageId: undefined,
         messageBatchReference: expect.any(String),
         messageReference: expect.any(String)
@@ -555,7 +558,7 @@ describe("NHS notify lambda helper functions", () => {
       expect(result).toMatchObject([
         {
           PSUDataItem: data[0].PSUDataItem,
-          success: false,
+          deliveryStatus: "notify request failed",
           notifyMessageId: undefined,
           messageBatchReference: expect.any(String),
           messageReference: expect.any(String)
@@ -603,7 +606,10 @@ describe("NHS notify lambda helper functions", () => {
       expect(result).toHaveLength(2)
       result.forEach((r) =>
         expect(r).toEqual(
-          expect.objectContaining({success: false, notifyMessageId: undefined})
+          expect.objectContaining({
+            deliveryStatus: "notify request failed",
+            notifyMessageId: undefined
+          })
         )
       )
       expect(errorSpy).toHaveBeenCalledWith(
@@ -704,7 +710,12 @@ describe("NHS notify lambda helper functions", () => {
     })
 
     it("uses a dummy call when the MAKE_REAL_NOTIFY_REQUESTS_PARAM is false", async () => {
-      process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM = "false"
+      mockGetParametersByName.mockImplementation(async () => Promise.resolve(
+        {
+          [process.env.NOTIFY_API_BASE_URL_PARAM!]: TEST_URL,
+          [process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM!]: "false"
+        }
+      ))
       const {makeBatchNotifyRequest: fn} = await import("../src/utils")
 
       const data = [
@@ -743,14 +754,14 @@ describe("NHS notify lambda helper functions", () => {
       expect(result).toHaveLength(2)
       expect(result[0]).toMatchObject({
         PSUDataItem: data[0].PSUDataItem,
-        success: true,
+        deliveryStatus: "silent running",
         notifyMessageId: expect.any(String), // it will be assigned a dummy ID
         messageBatchReference: expect.any(String),
         messageReference: expect.any(String)
       })
       expect(result[1]).toMatchObject({
         PSUDataItem: data[1].PSUDataItem,
-        success: true,
+        deliveryStatus: "silent running",
         notifyMessageId: expect.any(String),
         messageBatchReference: expect.any(String),
         messageReference: expect.any(String)

packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts

@@ -3,7 +3,7 @@ import {APIGatewayProxyEvent, APIGatewayProxyResult} from "aws-lambda"
 import {Logger} from "@aws-lambda-powertools/logger"
 import {injectLambdaContext} from "@aws-lambda-powertools/logger/middleware"
 import {TransactionCanceledException} from "@aws-sdk/client-dynamodb"
-import {getParameter} from "@aws-lambda-powertools/parameters/ssm"
+import {SSMProvider} from "@aws-lambda-powertools/parameters/ssm"
 
 import middy from "@middy/core"
 import inputOutputLogger from "@middy/input-output-logger"
@@ -45,7 +45,22 @@ export const TEST_PRESCRIPTIONS_1 = (process.env.TEST_PRESCRIPTIONS_1 ?? "")
 export const TEST_PRESCRIPTIONS_2 = (process.env.TEST_PRESCRIPTIONS_2 ?? "")
   .split(",").map(item => item.trim()) || []
 
-const ENABLE_NOTIFICATIONS_PARAM = process.env.ENABLE_NOTIFICATIONS_PARAM
+// Fetching the parameters from SSM using a dedicated provider, so that the values can be cached
+// and reused across invocations, reducing the number of calls to SSM.
+// (it was failing load tests using getParameter directly)
+const ssm = new SSMProvider()
+const paramNames = {
+  [process.env.ENABLE_NOTIFICATIONS_PARAM!]: {maxAge: 60}
+}
+const configPromise = ssm.getParametersByName(paramNames)
+
+async function loadConfig() {
+  const all = await configPromise
+  console.log(all)
+  return {
+    enableNotifications: all[process.env.ENABLE_NOTIFICATIONS_PARAM!] === "true"
+  }
+}
 
 const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> => {
   logger.appendKeys({
@@ -66,6 +81,8 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
     "x-request-id": xRequestID
   })
 
+  const paramPromise = loadConfig()
+
   const requestBody = event.body
   const requestBundle = castEventBody(requestBody, responseEntries)
   if (!requestBundle) {
@@ -115,16 +132,17 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
 
   await logTransitions(dataItems)
 
-  // This prescription update is valid and all seems alright, so request for a notification
-  let enableNotifications: boolean = false
+  // Await the parameter promise before we continue
+  let enableNotificationsFlag = false
   try {
-    if (ENABLE_NOTIFICATIONS_PARAM) enableNotifications = await getParameter(ENABLE_NOTIFICATIONS_PARAM) === "true"
-  } catch {
-    logger.error("Failed to fetch ENABLE_NOTIFICATIONS_PARAM. Defaulting to false.", {ENABLE_NOTIFICATIONS_PARAM})
+    const {enableNotifications} = await paramPromise
+    enableNotificationsFlag = enableNotifications
+  } catch (err) {
+    logger.error("Failed to load parameters from SSM", {err})
   }
 
   let created_messageIds: Array<string> = []
-  if (enableNotifications) {
+  if (enableNotificationsFlag) {
     try {
       const requestId = event.headers["x-request-id"] ?? "x-request-id-not-found"
       created_messageIds = await pushPrescriptionToNotificationSQS(requestId, dataItems, logger)
@@ -135,7 +153,7 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
   } else {
     logger.info(
       "enableNotifications is not true, skipping the notification request.",
-      {enableNotifications}
+      {enableNotificationsFlag}
     )
   }
 

packages/updatePrescriptionStatus/src/utils/sqsClient.ts

@@ -176,15 +176,15 @@ export async function pushPrescriptionToNotificationSQS(
         Entries: batch
       })
       const result = await sqs.send(command)
-      if (result.Successful && result.Successful.length) {
+      if (result.Successful?.length) {
         logger.info("Successfully sent a batch of prescriptions to the notifications SQS", {result})
 
         // For each successful message, get its message ID. I don't think there will ever be undefined
         // actually in here, but the typing suggests that there could be so filter those out
         out.push(...result.Successful.map(e => e.MessageId).filter(msg_id => msg_id !== undefined))
       }
       // Some may succeed, and some may fail. So check for both
-      if (result.Failed && result.Failed.length) {
+      if (result.Failed?.length) {
         throw new Error("Failed to send a batch of prescriptions to the notifications SQS")
       }
     } catch (error) {