CCM-8861: Changes in response to review comments

Author: chris-elliott-nhsd

infrastructure/terraform/modules/backend-api/module_lambda_sftp_poll.tf

@@ -1,6 +1,6 @@
 module "lambda_sftp_poll" {
   source      = "../lambda-function"
-  description = "Lambda to poll the SFTP suppliers and "
+  description = "Lambda to poll the SFTP suppliers and copy proofs to the quarantine bucket"
 
   function_name    = "${local.csi}-sftp-poll"
   filename         = module.build_sftp_letters_lambdas.zips["src/sftp-poll.ts"].path
@@ -20,7 +20,8 @@ module "lambda_sftp_poll" {
     SFTP_ENVIRONMENT        = local.sftp_environment
   }
 
-  timeout = 20
+  timeout     = 60 * 10
+  memory_size = 2048
 }
 
 data "aws_iam_policy_document" "sftp_poll" {

lambdas/sftp-letters/package.json

@@ -1,12 +1,18 @@
 {
-  "name": "nhs-notify-sftp-letters-lambdas",
-  "version": "0.0.1",
-  "private": true,
-  "scripts": {
-    "test:unit": "jest",
-    "lint": "eslint .",
-    "lint:fix": "eslint . --fix",
-    "typecheck": "tsc --noEmit"
+  "dependencies": {
+    "@aws-sdk/client-dynamodb": "3.775.0",
+    "@aws-sdk/client-s3": "3.775.0",
+    "@aws-sdk/client-ssm": "3.775.0",
+    "@aws-sdk/lib-dynamodb": "3.775.0",
+    "csv-parse": "^5.6.0",
+    "csv-stringify": "^6.4.4",
+    "date-fns": "^4.1.0",
+    "ksuid": "^3.0.0",
+    "nhs-notify-entity-update-command-builder": "*",
+    "nhs-notify-web-template-management-utils": "*",
+    "node-cache": "^5.1.2",
+    "ssh2-sftp-client": "^9.1.0",
+    "zod": "^3.24.2"
   },
   "devDependencies": {
     "@swc/core": "^1.11.3",
@@ -20,24 +26,18 @@
     "esbuild": "^0.24.0",
     "jest": "^29.7.0",
     "jest-mock-extended": "^3.0.7",
+    "nhs-notify-web-template-management-test-helper-utils": "*",
     "ts-jest": "^29.3.0",
     "ts-node": "^10.9.2",
     "typescript": "^5.8.2"
   },
-  "dependencies": {
-    "@aws-sdk/client-dynamodb": "3.775.0",
-    "@aws-sdk/client-s3": "3.775.0",
-    "@aws-sdk/client-ssm": "3.775.0",
-    "@aws-sdk/lib-dynamodb": "3.775.0",
-    "csv-parse": "^5.6.0",
-    "csv-stringify": "^6.4.4",
-    "date-fns": "^4.1.0",
-    "ksuid": "^3.0.0",
-    "nhs-notify-entity-update-command-builder": "*",
-    "nhs-notify-web-template-management-test-helper-utils": "*",
-    "nhs-notify-web-template-management-utils": "*",
-    "node-cache": "^5.1.2",
-    "ssh2-sftp-client": "^9.1.0",
-    "zod": "^3.24.2"
-  }
+  "name": "nhs-notify-sftp-letters-lambdas",
+  "private": true,
+  "scripts": {
+    "lint": "eslint .",
+    "lint:fix": "eslint . --fix",
+    "test:unit": "jest",
+    "typecheck": "tsc --noEmit"
+  },
+  "version": "0.0.1"
 }

lambdas/sftp-letters/src/__tests__/app/poll.test.ts

@@ -3,8 +3,8 @@ import { mockDeep } from 'jest-mock-extended';
 import { App } from '../../app/poll';
 import { SftpSupplierClientRepository } from '../../infra/sftp-supplier-client-repository';
 import { SftpClient } from '../../infra/sftp-client';
-import { Logger } from 'nhs-notify-web-template-management-utils/logger';
 import { S3Repository } from 'nhs-notify-web-template-management-utils';
+import { createMockLogger } from 'nhs-notify-web-template-management-test-helper-utils/mock-logger';
 
 const mockPdfBuffer = Buffer.from([0x25, 0x50, 0x44, 0x46]);
 const mockMalformedPdfBuffer = Buffer.from([]);
@@ -13,7 +13,12 @@ const downloadError = new Error('Download error');
 const directoryType: FileInfo['type'] = 'd';
 const fileType: FileInfo['type'] = '-';
 
-test('polls SFTP clients', async () => {
+beforeAll(() => {
+  jest.useFakeTimers();
+  jest.setSystemTime(new Date('2022-01-01 09:00'));
+});
+
+test('polls SFTP folder', async () => {
   const sftpClient = mockDeep<SftpClient>({
     exists: async (path: string) => {
       const existsMappings: Record<string, FileInfo['type']> = {
@@ -56,7 +61,7 @@ test('polls SFTP clients', async () => {
             modifyTime: Date.now(),
           },
           {
-            name: 'template-3.pdf',
+            name: 'file-to-ignore.pdf',
             type: fileType,
             modifyTime: Date.now(),
           },
@@ -87,6 +92,11 @@ test('polls SFTP clients', async () => {
             type: fileType,
             modifyTime: Date.now(),
           },
+          {
+            name: 'extra-folder-to-ignore',
+            type: directoryType,
+            modifyTime: Date.now(),
+          },
         ],
       })[path] ?? [],
   });
@@ -100,19 +110,19 @@ test('polls SFTP clients', async () => {
     }),
   });
 
-  const mockLogger = mockDeep<Logger>();
+  const mockLogger = createMockLogger();
   const s3Repository = mockDeep<S3Repository>();
 
   const app = new App(
     sftpSupplierClientRepository,
-    mockLogger,
+    mockLogger.logger,
     s3Repository,
     'sftp-environment'
   );
 
   await app.poll('supplier');
 
-  expect(s3Repository.putRawData).toHaveBeenCalledTimes(3);
+  expect(s3Repository.putRawData).toHaveBeenCalledTimes(2);
   expect(s3Repository.putRawData).toHaveBeenCalledWith(
     mockPdfBuffer,
     'proofs/template-1-folder/template-1.pdf'
@@ -121,21 +131,66 @@ test('polls SFTP clients', async () => {
     mockPdfBuffer,
     'proofs/template-1-folder/template-2.pdf'
   );
-  expect(s3Repository.putRawData).toHaveBeenCalledWith(
-    mockPdfBuffer,
-    'proofs/template-3.pdf'
-  );
 
-  expect(mockLogger.error).toHaveBeenCalledWith('PDF file failed validation', {
-    copyPath:
-      'download-dir/sftp-environment/proofs/template-1-folder/invalid-file.pdf',
-    pastePath: 'proofs/template-1-folder/invalid-file.pdf',
+  expect(mockLogger.logMessages).toContainEqual({
+    level: 'error',
+    message: {
+      description: 'PDF file failed validation',
+      sftpPath:
+        'download-dir/sftp-environment/proofs/template-1-folder/invalid-file.pdf',
+      s3Path: 'proofs/template-1-folder/invalid-file.pdf',
+    },
+    timestamp: new Date('2022-01-01 09:00').toISOString(),
   });
-  expect(mockLogger.error).toHaveBeenCalledWith('Failed to copy file', {
-    copyPath:
+  expect(mockLogger.logMessages).toContainEqual({
+    level: 'error',
+    description: 'Failed to process file',
+    sftpPath:
       'download-dir/sftp-environment/proofs/template-1-folder/download-error.pdf',
-    pastePath: 'proofs/template-1-folder/download-error.pdf',
-    error: downloadError,
+    s3Path: 'proofs/template-1-folder/download-error.pdf',
+    timestamp: new Date('2022-01-01 09:00').toISOString(),
+    stack: expect.stringContaining('Error: Download error'),
+    message: 'Download error',
+  });
+
+  expect(sftpClient.connect).toHaveBeenCalledTimes(1);
+  expect(sftpClient.end).toHaveBeenCalledTimes(1);
+});
+
+test('attempts to poll folder that does not exist', async () => {
+  const mockSftpList = jest.fn();
+  const sftpClient = mockDeep<SftpClient>({
+    exists: async () => false as const,
+    list: mockSftpList,
+  });
+
+  const sftpSupplierClientRepository = mockDeep<SftpSupplierClientRepository>({
+    getClient: async () => ({
+      sftpClient,
+      baseUploadDir: 'upload-dir',
+      baseDownloadDir: 'download-dir',
+      name: 'supplier',
+    }),
+  });
+
+  const mockLogger = createMockLogger();
+  const s3Repository = mockDeep<S3Repository>();
+
+  const app = new App(
+    sftpSupplierClientRepository,
+    mockLogger.logger,
+    s3Repository,
+    'sftp-environment'
+  );
+
+  await app.poll('supplier');
+
+  expect(sftpClient.list).not.toHaveBeenCalled();
+
+  expect(mockLogger.logMessages).toContainEqual({
+    level: 'info',
+    message: "Path 'download-dir/sftp-environment/proofs' does not exist",
+    timestamp: new Date('2022-01-01 09:00').toISOString(),
   });
 
   expect(sftpClient.connect).toHaveBeenCalledTimes(1);

lambdas/sftp-letters/src/api/sftp-poll.ts

@@ -1,10 +1,10 @@
 import type { Container } from '../container-poll';
 
-type EventBridgeEvent = {
+type PollEvent = {
   supplier: string;
 };
 
 export const createHandler =
   ({ app }: Container) =>
-  ({ supplier }: EventBridgeEvent) =>
+  ({ supplier }: PollEvent) =>
     app.poll(supplier);