CCM-16553: Code review comments

Author: nhsd-angel-pastor

package-lock.json

@@ -1,31 +1,14 @@
-import type { Config } from 'jest';
+import { baseJestConfig } from '../jest.config.base';
 
-const config: Config = {
-  preset: 'ts-jest',
-  testEnvironment: 'node',
+const config = {
+  ...baseJestConfig,
   roots: ['<rootDir>/nft-event-generator'],
-  testMatch: ['**/__tests__/**/*.test.ts', '**/?(*.)+(spec|test).ts'],
-  transform: {
-    '^.+\\.ts$': [
-      'ts-jest',
-      {
-        tsconfig: {
-          esModuleInterop: true,
-          allowSyntheticDefaultImports: true,
-          allowImportingTsExtensions: true,
-          module: 'commonjs',
-          target: 'ES2020',
-          moduleResolution: 'node',
-          noEmit: true,
-          typeRoots: ['../node_modules/@types'],
-        },
-        diagnostics: {
-          ignoreCodes: [1343], // Ignore TS1343: import.meta errors
-        },
-      },
-    ],
-  },
   modulePaths: ['<rootDir>/nft-event-generator/src'],
+  moduleNameMapper: {
+    // Map local subpath aliases that would otherwise resolve to the 'utils'
+    // workspace package before the local nft-event-generator source is checked.
+    '^utils/(.+)$': '<rootDir>/nft-event-generator/src/utils/$1',
+  },
   collectCoverageFrom: [
     'nft-event-generator/src/**/*.{ts,js}',
     '!nft-event-generator/src/**/*.d.ts',
@@ -35,21 +18,14 @@ const config: Config = {
     '!nft-event-generator/src/cli.ts',
   ],
   coverageDirectory: 'nft-event-generator/coverage',
-  coveragePathIgnorePatterns: ['/node_modules/', '/__tests__/'],
   coverageThreshold: {
     global: {
-      branches: 60,
-      functions: 60,
-      lines: 60,
-      statements: 60,
+      branches: 80,
+      functions: 80,
+      lines: 80,
+      statements: 80,
     },
   },
-  moduleFileExtensions: ['ts', 'js', 'json'],
-  moduleNameMapper: {
-    '^(.*)\\.ts$': '$1',
-  },
-  setupFilesAfterEnv: ['<rootDir>/jest.setup.ts'],
-  testTimeout: 10_000,
 };
 
 export default config;

scripts/jest.config.ts

@@ -99,20 +99,6 @@ npm start -- paper-letter-opt-out-event --environment pr293 --csvFile ./opt-outs
 
 ---
 
-## Running via Make
-
-To run this script from anywhere in the repository:
-
-```shell
-make perf-test
-```
-
-The make command runs the following script (configured in `package.json`):
-
-```shell
-"start:nft": "npm start -- supplier-api-letter-event --environment nft --numberOfEvents 2 --interval 2000"
-```
-
 ## Help
 
 To see all available options for a subcommand:

scripts/nft-event-generator/README.md

@@ -1,11 +1,15 @@
 import {
+  EventBridgeClient,
   PutEventsCommand,
   PutEventsResultEntry,
 } from '@aws-sdk/client-eventbridge';
 import { PublishableEvent } from 'destinations/destination-client';
 import { mock } from 'jest-mock-extended';
-import { sendEventsToEventBus } from 'destinations/send-events-to-event-bus';
-/* eslint-disable no-console */
+import {
+  EventBusDestinationClient,
+  sendEventsToEventBus,
+} from 'destinations/send-events-to-event-bus';
+
 const environment = 'dev';
 
 const sampleEvent = {
@@ -15,33 +19,39 @@ const sampleEvent = {
   time: '2023-06-20T12:00:00.000Z',
 };
 
-const mockEventBridgeClient = { send: jest.fn() };
+// Factory no longer references any outer variable
 jest.mock('@aws-sdk/client-eventbridge', () => {
   const originalModule = jest.requireActual('@aws-sdk/client-eventbridge');
-
   return {
     __esModule: true,
     ...originalModule,
-    EventBridgeClient: jest.fn(() => mockEventBridgeClient),
+    EventBridgeClient: jest.fn(),
   };
 });
 
+const mockSend = jest.fn();
+
 const successEntry = mock<PutEventsResultEntry>({ ErrorCode: undefined });
 const successfulSendResponse = { Entries: [successEntry] };
 
 describe('sendEventsToEventBus', () => {
   beforeEach(() => {
-    mockEventBridgeClient.send.mockReset();
+    mockSend.mockReset();
+    // Wire up the mock implementation here, after all declarations are initialised
+    jest
+      .mocked(EventBridgeClient)
+      .mockImplementation(
+        () => ({ send: mockSend }) as unknown as EventBridgeClient,
+      );
   });
 
   it('should send the expected request to EventBridge', async () => {
-    mockEventBridgeClient.send.mockResolvedValue(successfulSendResponse);
+    mockSend.mockResolvedValue(successfulSendResponse);
 
     await sendEventsToEventBus(environment, [sampleEvent], 5);
 
-    expect(mockEventBridgeClient.send).toHaveBeenCalled();
-    const putEventsCommand: PutEventsCommand =
-      mockEventBridgeClient.send.mock.calls[0][0];
+    expect(mockSend).toHaveBeenCalled();
+    const putEventsCommand: PutEventsCommand = mockSend.mock.calls[0][0];
 
     expect(putEventsCommand.input.Entries).toHaveLength(1);
     const entry = putEventsCommand.input.Entries![0];
@@ -56,19 +66,17 @@ describe('sendEventsToEventBus', () => {
       { length: 52 },
       () => sampleEvent,
     );
-    mockEventBridgeClient.send.mockResolvedValue(successfulSendResponse);
+    mockSend.mockResolvedValue(successfulSendResponse);
 
     await sendEventsToEventBus(environment, events, 5);
 
     // Batch size is 10, so 52 events = 6 batches.
-    expect(mockEventBridgeClient.send).toHaveBeenCalledTimes(6);
+    expect(mockSend).toHaveBeenCalledTimes(6);
   });
 
   it('should continue sending batches if an error is raised', async () => {
-    mockEventBridgeClient.send.mockRejectedValueOnce(
-      new Error('Something went wrong!'),
-    );
-    mockEventBridgeClient.send.mockResolvedValue(successfulSendResponse);
+    mockSend.mockRejectedValueOnce(new Error('Something went wrong!'));
+    mockSend.mockResolvedValue(successfulSendResponse);
 
     const events: PublishableEvent[] = Array.from(
       { length: 30 },
@@ -78,19 +86,44 @@ describe('sendEventsToEventBus', () => {
     await sendEventsToEventBus(environment, events, 5);
 
     // Batch size is 10, so 30 events = 3 batches.
-    expect(mockEventBridgeClient.send).toHaveBeenCalledTimes(3);
+    expect(mockSend).toHaveBeenCalledTimes(3);
   });
 
   it('should warn when some events fail to publish', async () => {
+    const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
     const failedEntry = mock<PutEventsResultEntry>({
       ErrorCode: 'InternalFailure',
     });
-    mockEventBridgeClient.send.mockResolvedValue({
+    mockSend.mockResolvedValue({
       Entries: [failedEntry],
     });
 
     await sendEventsToEventBus(environment, [sampleEvent], 5);
 
-    expect(console.warn).toHaveBeenCalled();
+    expect(warnSpy).toHaveBeenCalled();
+    warnSpy.mockRestore();
+  });
+});
+
+describe('EventBusDestinationClient', () => {
+  beforeEach(() => {
+    mockSend.mockReset();
+    jest
+      .mocked(EventBridgeClient)
+      .mockImplementation(
+        () => ({ send: mockSend }) as unknown as EventBridgeClient,
+      );
+  });
+
+  it('should delegate sendEvents to sendEventsToEventBus', async () => {
+    mockSend.mockResolvedValue(successfulSendResponse);
+
+    const client = new EventBusDestinationClient(environment);
+    await client.sendEvents([sampleEvent], 5);
+
+    expect(mockSend).toHaveBeenCalledTimes(1);
+    const putEventsCommand: PutEventsCommand = mockSend.mock.calls[0][0];
+    const entry = putEventsCommand.input.Entries![0];
+    expect(entry.EventBusName).toBe(`nhs-${environment}-dl`);
   });
 });

scripts/nft-event-generator/src/__tests__/destinations/send-events-to-event-bus.test.ts

@@ -31,7 +31,7 @@ describe('generatePaperLetterOptOutEvents', () => {
         'customer/037f5f76-352c-445f-89a7-c3d18776ce86/message/3COesqsClaLyf0WNuLuhz1RDbWs/plan/3COezubdtrUFJlDOV4ucAQ93Akr',
       id: expect.any(String),
       time: expect.any(String),
-      source: '/nhs/england/notify/comms-mgr-test/test/data-plane/messaging',
+      source: `/nhs/england/notify/comms-mgr-${environment}/${environment}/data-plane/messaging`,
       data: {
         messageReference: sampleRows[0].messageReference,
         clientId: sampleRows[0].senderId,

scripts/nft-event-generator/src/__tests__/generators/generate-paper-letter-opt-out-events.test.ts

@@ -1,9 +1,14 @@
 /**
  * Minimum shape required by all destination clients.
- * Each publishable event must carry an `id` so the sending implementation
- * can use it as a unique identifier within a batch.
+ * Each publishable event must carry the defined properties, where the `id`
+ * is used as a unique identifier within a batch.
  */
-export type PublishableEvent = { id: string };
+export type PublishableEvent = {
+  id: string;
+  source: string;
+  type: string;
+  time: string;
+};
 
 /**
  * Common interface for all event destinations (SQS, EventBridge, etc.).

scripts/nft-event-generator/src/destinations/destination-client.ts

@@ -6,6 +6,7 @@ import {
   DestinationClient,
   PublishableEvent,
 } from 'destinations/destination-client';
+import { sleep } from 'utils';
 
 /* eslint-disable no-console */
 
@@ -32,13 +33,6 @@ function batchEvents(events: PublishableEvent[]): PublishableEvent[][] {
   return batches;
 }
 
-// Wait for X milliseconds
-function wait(interval: number) {
-  return new Promise((resolve) => {
-    setTimeout(resolve, interval);
-  });
-}
-
 export async function sendEventsToEventBus(
   environment: string,
   events: PublishableEvent[],
@@ -59,7 +53,7 @@ export async function sendEventsToEventBus(
     currentBatch += 1;
 
     const entries = batch.map((event) => {
-      const cloudEvent = event as unknown as CloudEventEnvelope;
+      const cloudEvent = event as CloudEventEnvelope;
       return {
         EventBusName: eventBusName,
         Source: cloudEvent.source,
@@ -91,7 +85,7 @@ export async function sendEventsToEventBus(
 
     // Wait before sending the next batch, but skip waiting after the last batch
     if (batch !== batches.at(-1)) {
-      await wait(interval);
+      await sleep(interval / 1000);
     }
   }
 

scripts/nft-event-generator/src/destinations/send-events-to-event-bus.ts

@@ -29,7 +29,7 @@ function generatePaperLetterOptOutEvent(
       'customer/037f5f76-352c-445f-89a7-c3d18776ce86/message/3COesqsClaLyf0WNuLuhz1RDbWs/plan/3COezubdtrUFJlDOV4ucAQ93Akr',
     id: randomUUID(),
     time: new Date().toISOString(),
-    source: '/nhs/england/notify/comms-mgr-test/test/data-plane/messaging',
+    source: `/nhs/england/notify/comms-mgr-${environment}/${environment}/data-plane/messaging`,
     data: {
       messageReference,
       clientId: senderId,

scripts/nft-event-generator/src/generators/paper-letter-opt-out-events.ts

@@ -45,6 +45,7 @@ function generateSupplierApiLetterEvent(
     time?: string;
     subject?: string;
     messageReference?: string;
+    /* istanbul ignore next */
   } = {},
 ): LetterEvent {
   const now = new Date();
@@ -96,18 +97,14 @@ export function generateSupplierApiLetterEvents({
   subject,
   time,
 }: GenerateEventsParams): LetterEvent[] {
-  const events: LetterEvent[] = [];
-
-  for (let i = 0; i < numberOfEvents; i++) {
-    events.push(
-      generateSupplierApiLetterEvent(environment, status, {
-        id,
-        time,
-        subject,
-        messageReference,
-      }),
-    );
-  }
+  const events = Array.from({ length: numberOfEvents }, () =>
+    generateSupplierApiLetterEvent(environment, status, {
+      id,
+      time,
+      subject,
+      messageReference,
+    }),
+  );
 
   console.group('Event generation:');
   console.log(`Total events generated:\t\t${numberOfEvents}`);

scripts/nft-event-generator/src/generators/supplier-api-letter-events.ts

@@ -9,14 +9,9 @@ export type PaperLetterOptOutRow = {
 export function readCsvFile(filePath: string): PaperLetterOptOutRow[] {
   const fileContent = readFileSync(filePath, 'utf8');
 
-  const records = parse(fileContent, {
-    columns: false,
+  return parse(fileContent, {
+    columns: ['messageReference', 'senderId'],
     skip_empty_lines: true,
     trim: true,
   });
-
-  return records.map((row) => ({
-    messageReference: row[0],
-    senderId: row[1],
-  }));
 }