Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import { NotifyMessageProcessor } from 'app/notify-message-processor';
import { ISenderManagement } from 'sender-management';
import { SqsHandlerDependencies, createHandler } from 'apis/sqs-handler';
import { parseSqsRecord } from 'app/parse-sqs-message';
import { InvalidPdmResourceAvailableEvent } from 'domain/invalid-pdm-resource-available-event';
import { RequestNotifyError } from 'domain/request-notify-error';
import { validPdmEvent, validSender } from '__tests__/constants';
import {
InvalidEvent,
MessageRequestRejected,
MessageRequestSkipped,
MessageRequestSubmitted,
Expand Down Expand Up @@ -195,14 +195,14 @@ describe('createHandler', () => {
});
});

describe('when parseSqsRecord throws InvalidPdmResourceAvailableEvent', () => {
describe('when parseSqsRecord throws InvalidEvent', () => {
it('marks the message as failed for retry', async () => {
const sqsEvent = createSqsEvent(1);
const handler = createHandler(dependencies);
const { messageId } = sqsEvent.Records[0];

mockParseSqsRecord.mockImplementationOnce(() => {
throw new InvalidPdmResourceAvailableEvent(messageId);
throw new InvalidEvent('Some validation errors');
});

const result = await handler(sqsEvent);
Expand All @@ -211,7 +211,7 @@ describe('createHandler', () => {
batchItemFailures: [{ itemIdentifier: messageId }],
});
expect(mockLogger.warn).toHaveBeenCalledWith({
error: 'Unable to parse PDMResourceAvailable event from SQS message',
error: 'Unable to parse event',
description: 'Failed processing message',
messageId,
senderId: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import type { SQSRecord } from 'aws-lambda';
import { mock } from 'jest-mock-extended';
import { Logger } from 'utils';
import { parseSqsRecord } from 'app/parse-sqs-message';
import { InvalidPdmResourceAvailableEvent } from 'domain/invalid-pdm-resource-available-event';
import { validPdmEvent } from '__tests__/constants';
import { InvalidEvent } from 'digital-letters-events';

const mockLogger = mock<Logger>();
const mockChildLogger = mock<Logger>();

describe('parseSqsRecord', () => {
const messageId = 'test-message-id-123';
Expand All @@ -27,6 +28,10 @@ describe('parseSqsRecord', () => {
awsRegion: 'eu-west-2',
});

beforeAll(() => {
mockLogger.child.mockReturnValue(mockChildLogger);
});

beforeEach(() => {
jest.clearAllMocks();
});
Expand All @@ -38,13 +43,12 @@ describe('parseSqsRecord', () => {
const result = parseSqsRecord(sqsRecord, mockLogger);

expect(result).toEqual(validPdmEvent);
expect(mockLogger.info).toHaveBeenCalledWith({
expect(mockLogger.child).toHaveBeenCalledWith({ messageId });
expect(mockChildLogger.info).toHaveBeenCalledWith({
description: 'Parsing SQS Record',
messageId,
Comment thread
simonlabarere marked this conversation as resolved.
});
expect(mockLogger.info).toHaveBeenCalledWith({
expect(mockChildLogger.info).toHaveBeenCalledWith({
description: 'Parsed valid PDMResourceAvailable Event',
messageId,
messageReference: validPdmEvent.data.messageReference,
senderId: validPdmEvent.data.senderId,
resourceId: validPdmEvent.data.resourceId,
Expand All @@ -53,20 +57,16 @@ describe('parseSqsRecord', () => {
});

describe('when SQS record contains an invalid PDMResourceAvailable event', () => {
it('logs error and throws InvalidPdmResourceAvailableEvent', () => {
it('logs error and throws InvalidEvent', () => {
const invalidEvent = { ...validPdmEvent, data: {} };
const sqsRecord = createSqsRecord(invalidEvent);

expect(() => parseSqsRecord(sqsRecord, mockLogger)).toThrow(
InvalidPdmResourceAvailableEvent,
);

expect(mockLogger.error).toHaveBeenCalledWith(
expect(() => parseSqsRecord(sqsRecord, mockLogger)).toThrow(InvalidEvent);
expect(mockLogger.child).toHaveBeenCalledWith({ messageId });
expect(mockChildLogger.error).toHaveBeenCalledWith(
expect.objectContaining({
description:
'The SQS message does not contain a valid PDMResourceAvailable event',
messageId,
error: expect.any(Array),
description: 'Error parsing PDMResourceAvailable event',
err: expect.any(Array),
}),
);
});
Expand All @@ -92,9 +92,8 @@ describe('parseSqsRecord', () => {
};

expect(() => parseSqsRecord(sqsRecord, mockLogger)).toThrow(SyntaxError);
expect(mockLogger.info).toHaveBeenCalledWith({
expect(mockChildLogger.info).toHaveBeenCalledWith({
description: 'Parsing SQS Record',
messageId,
});
});
});
Expand Down
12 changes: 6 additions & 6 deletions lambdas/core-notifier-lambda/src/apis/sqs-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ import {
MessageRequestSkipped,
MessageRequestSubmitted,
PDMResourceAvailable,
validateMessageRequestRejected,
validateMessageRequestSkipped,
validateMessageRequestSubmitted,
} from 'digital-letters-events';
import {
mapPdmEventToMessageRequestRejected,
mapPdmEventToMessageRequestSkipped,
mapPdmEventToMessageRequestSubmitted,
mapPdmEventToSingleMessageRequest,
} from 'domain/mapper';
import messageRequestSubmittedValidator from 'digital-letters-events/MessageRequestSubmitted.js';
import messageRequestRejectedValidator from 'digital-letters-events/MessageRequestRejected.js';
import messageRequestSkippedValidator from 'digital-letters-events/MessageRequestSkipped.js';
import { parseSqsRecord } from 'app/parse-sqs-message';

import type { NotifyMessageProcessor } from 'app/notify-message-processor';
Expand Down Expand Up @@ -186,17 +186,17 @@ export const createHandler = ({
submittedEvents.length > 0 &&
eventPublisher.sendEvents<MessageRequestSubmitted>(
submittedEvents,
messageRequestSubmittedValidator,
validateMessageRequestSubmitted,
),
skippedEvents.length > 0 &&
eventPublisher.sendEvents<MessageRequestSkipped>(
skippedEvents,
messageRequestSkippedValidator,
validateMessageRequestSkipped,
),
rejectedEvents.length > 0 &&
eventPublisher.sendEvents<MessageRequestRejected>(
rejectedEvents,
messageRequestRejectedValidator,
validateMessageRequestRejected,
),
].filter(Boolean),
);
Expand Down
24 changes: 8 additions & 16 deletions lambdas/core-notifier-lambda/src/app/parse-sqs-message.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,26 @@
import type { SQSRecord } from 'aws-lambda';
import { Logger } from 'utils';
import { PDMResourceAvailable } from 'digital-letters-events';
import { InvalidPdmResourceAvailableEvent } from 'domain/invalid-pdm-resource-available-event';
import messagePDMResourceAvailableValidator from 'digital-letters-events/PDMResourceAvailable.js';
import {
PDMResourceAvailable,
validatePDMResourceAvailable,
} from 'digital-letters-events';

export const parseSqsRecord = (
sqsRecord: SQSRecord,
logger: Logger,
): PDMResourceAvailable => {
logger.info({
const childLogger = logger.child({ messageId: sqsRecord.messageId });
childLogger.info({
description: 'Parsing SQS Record',
messageId: sqsRecord.messageId,
});

const sqsEventBody = JSON.parse(sqsRecord.body);
const sqsEventDetail = sqsEventBody.detail;

if (!messagePDMResourceAvailableValidator(sqsEventDetail)) {
logger.error({
error: messagePDMResourceAvailableValidator.errors,
description:
'The SQS message does not contain a valid PDMResourceAvailable event',
messageId: sqsRecord.messageId,
});
throw new InvalidPdmResourceAvailableEvent(sqsRecord.messageId);
}
validatePDMResourceAvailable(sqsEventDetail, childLogger);

logger.info({
childLogger.info({
description: 'Parsed valid PDMResourceAvailable Event',
messageId: sqsRecord.messageId,
messageReference: sqsEventDetail.data.messageReference,
senderId: sqsEventDetail.data.senderId,
resourceId: sqsEventDetail.data.resourceId,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ describe('SQS Handler', () => {
);
expect(mockLogger.warn).toHaveBeenCalledWith(
expect.objectContaining({
description: 'Error parsing queue entry',
description: 'Error parsing SQS record',
}),
);
});
Expand Down Expand Up @@ -405,7 +405,7 @@ describe('SQS Handler', () => {
expect(mockFileScanner.scanFile).not.toHaveBeenCalled();
expect(mockLogger.warn).toHaveBeenCalledWith(
expect.objectContaining({
description: 'Error parsing queue entry',
description: 'Error parsing SQS record',
}),
);
expect(mockLogger.warn).toHaveBeenCalledWith(
Expand Down
13 changes: 2 additions & 11 deletions lambdas/file-scanner-lambda/src/apis/sqs-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import type {
SQSBatchResponse,
SQSEvent,
} from 'aws-lambda';
import { ItemDequeued } from 'digital-letters-events';
import itemDequeuedValidator from 'digital-letters-events/ItemDequeued.js';
import { ItemDequeued, validateItemDequeued } from 'digital-letters-events';
import { EventPublisher, Logger } from 'utils';

export interface HandlerDependencies {
Expand All @@ -27,15 +26,7 @@ function validateRecord(
const sqsEventBody = JSON.parse(body);
const sqsEventDetail = sqsEventBody.detail;

const isEventValid = itemDequeuedValidator(sqsEventDetail);
if (!isEventValid) {
logger.warn({
err: itemDequeuedValidator.errors,
description: 'Error parsing queue entry',
});

return null;
}
Comment thread
simonlabarere marked this conversation as resolved.
validateItemDequeued(sqsEventDetail, logger);

return { messageId, event: sqsEventDetail };
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import {
validateFileQuarantined,
validateFileSafe,
} from 'digital-letters-events';
import { createFileQuarantinedEvent, createFileSafeEvent } from 'domain/mapper';
import fileSafeValidator from 'digital-letters-events/FileSafe.js';
import fileQuarantinedValidator from 'digital-letters-events/FileQuarantined.js';
import { mock } from 'jest-mock-extended';
import { Logger } from 'utils';

// Mock randomUUID to make tests deterministic
jest.mock('node:crypto', () => ({
Expand All @@ -22,6 +26,7 @@ describe('mapper', () => {
});

describe('createFileSafeEvent', () => {
const mockLogger = mock<Logger>();
it('creates a FileSafe event with correct structure', () => {
const messageReference = 'msg-ref-123';
const senderId = 'sender-456';
Expand Down Expand Up @@ -52,11 +57,7 @@ describe('mapper', () => {
recordedtime: '2024-01-15T10:30:00.000Z',
severitynumber: 2,
});
const isValid = fileSafeValidator(result);
if (!isValid) {
throw new Error(JSON.stringify(fileSafeValidator.errors, null, 2));
}
expect(isValid).toBe(true);
expect(() => validateFileSafe(result, mockLogger)).not.toThrow();
});

it('handles different input values correctly', () => {
Expand All @@ -80,6 +81,8 @@ describe('mapper', () => {
});

describe('createFileQuarantinedEvent', () => {
const mockLogger = mock<Logger>();

it('creates a FileQuarantined event with correct structure', () => {
const messageReference = 'msg-ref-789';
const senderId = 'sender-012';
Expand Down Expand Up @@ -111,13 +114,7 @@ describe('mapper', () => {
recordedtime: '2024-01-15T10:30:00.000Z',
severitynumber: 2,
});
const isValid = fileQuarantinedValidator(result);
if (!isValid) {
throw new Error(
JSON.stringify(fileQuarantinedValidator.errors, null, 2),
);
}
expect(isValid).toBe(true);
expect(() => validateFileQuarantined(result, mockLogger)).not.toThrow();
});
});
});
16 changes: 8 additions & 8 deletions lambdas/move-scanned-files-lambda/src/apis/sqs-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ import type {
SQSRecord,
} from 'aws-lambda';
import { EventPublisher, Logger } from 'utils';
import { FileQuarantined, FileSafe } from 'digital-letters-events';
import fileSafeValidator from 'digital-letters-events/FileSafe.js';
import fileQuarantinedValidator from 'digital-letters-events/FileQuarantined.js';
import {
FileQuarantined,
FileSafe,
validateFileQuarantined,
validateFileSafe,
} from 'digital-letters-events';
import { parseSqsRecord } from 'app/parse-sqs-message';
import { MoveFileHandler } from 'app/move-file-handler';

Expand Down Expand Up @@ -69,14 +72,11 @@ export const createHandler = ({
await Promise.all(
[
fileSafeEvents.length > 0 &&
eventPublisher.sendEvents<FileSafe>(
fileSafeEvents,
fileSafeValidator,
),
eventPublisher.sendEvents<FileSafe>(fileSafeEvents, validateFileSafe),
fileQuarantinedEvents.length > 0 &&
eventPublisher.sendEvents<FileQuarantined>(
fileQuarantinedEvents,
fileQuarantinedValidator,
validateFileQuarantined,
),
].filter(Boolean),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,13 @@ describe('SQS Handler', () => {

const result = await handler(event);

expect(logger.warn).toHaveBeenCalledWith({
expect(logger.error).toHaveBeenCalledWith({
err: expect.arrayContaining([
expect.objectContaining({
instancePath: '/source',
}),
]),
description: 'Error parsing queue entry',
description: 'Error parsing PDMResourceSubmitted event',
});

expect(logger.info).toHaveBeenCalledWith(
Expand All @@ -323,13 +323,13 @@ describe('SQS Handler', () => {

const result = await handler(event);

expect(logger.warn).toHaveBeenCalledWith({
expect(logger.error).toHaveBeenCalledWith({
err: expect.arrayContaining([
expect.objectContaining({
instancePath: '/source',
}),
]),
description: 'Error parsing queue entry',
description: 'Error parsing PDMResourceUnavailable event',
});

expect(logger.info).toHaveBeenCalledWith(
Expand Down
Loading
Loading