Skip to content
Open
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 @@ -35,7 +35,8 @@ module "supplier_allocator" {
log_subscription_role_arn = local.acct.log_subscription_role_arn

lambda_env_vars = merge(local.common_lambda_env_vars, {
UPSERT_LETTERS_QUEUE_URL = module.sqs_letter_updates.sqs_queue_url
UPSERT_LETTERS_QUEUE_URL = module.sqs_letter_updates.sqs_queue_url,
IDEMPOTENCY_TABLE_NAME = aws_dynamodb_table.idempotency.name
})
}

Expand Down Expand Up @@ -110,6 +111,7 @@ data "aws_iam_policy_document" "supplier_allocator_lambda" {

resources = [
aws_dynamodb_table.supplier-quotas.arn,
aws_dynamodb_table.idempotency.arn,
"${aws_dynamodb_table.supplier-quotas.arn}/index/*"
]
}
Expand Down
1 change: 1 addition & 0 deletions lambdas/supplier-allocator/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"dependencies": {
"@aws-lambda-powertools/idempotency": "^2.33.0",
"@aws-sdk/client-dynamodb": "^3.858.0",
"@aws-sdk/client-sqs": "^3.984.0",
"@aws-sdk/lib-dynamodb": "^3.1008.0",
Expand Down
13 changes: 13 additions & 0 deletions lambdas/supplier-allocator/src/config/__tests__/deps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ describe("createDependenciesContainer", () => {
const env = {
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
IDEMPOTENCY_TABLE_NAME: "IdempotencyTable",
};

beforeEach(() => {
Expand All @@ -27,6 +28,10 @@ describe("createDependenciesContainer", () => {
SupplierQuotasRepository: jest.fn(),
}));

jest.mock("@aws-lambda-powertools/idempotency/dynamodb", () => ({
DynamoDBPersistenceLayer: jest.fn(),
}));

// Env
jest.mock("../env", () => ({ envVars: env }));
});
Expand All @@ -40,6 +45,9 @@ describe("createDependenciesContainer", () => {
const { SupplierQuotasRepository } = jest.requireMock(
"@internal/datastore",
);
const { DynamoDBPersistenceLayer } = jest.requireMock(
"@aws-lambda-powertools/idempotency/dynamodb",
);
// eslint-disable-next-line @typescript-eslint/no-require-imports
const { createDependenciesContainer } = require("../deps");
const deps: Deps = createDependenciesContainer();
Expand All @@ -54,6 +62,11 @@ describe("createDependenciesContainer", () => {
expect(supplierQuotasRepoCtorArgs[1]).toEqual({
supplierQuotasTableName: "SupplierQuotasTable",
});
expect(DynamoDBPersistenceLayer).toHaveBeenCalledTimes(1);
const idempotencyLayerCtorArgs = DynamoDBPersistenceLayer.mock.calls[0][0];
expect(idempotencyLayerCtorArgs).toEqual({
tableName: "IdempotencyTable",
});
expect(deps.env).toEqual(env);
});
});
2 changes: 2 additions & 0 deletions lambdas/supplier-allocator/src/config/__tests__/env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ describe("lambdaEnv", () => {
it("should load all environment variables successfully", () => {
process.env.SUPPLIER_CONFIG_TABLE_NAME = "SupplierConfigTable";
process.env.SUPPLIER_QUOTAS_TABLE_NAME = "SupplierQuotasTable";
process.env.IDEMPOTENCY_TABLE_NAME = "IdempotencyTable";

const { envVars } = require("../env");

expect(envVars).toEqual({
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
IDEMPOTENCY_TABLE_NAME: "IdempotencyTable",
});
});
});
25 changes: 13 additions & 12 deletions lambdas/supplier-allocator/src/config/deps.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb";
import { DynamoDBPersistenceLayer } from "@aws-lambda-powertools/idempotency/dynamodb";
import { SQSClient } from "@aws-sdk/client-sqs";
import { Logger } from "pino";
import { createLogger } from "@internal/helpers";
Expand All @@ -12,6 +13,7 @@ import { EnvVars, envVars } from "./env";
export type Deps = {
supplierConfigRepo: SupplierConfigRepository;
supplierQuotasRepo: SupplierQuotasRepository;
idempotencyLayer: DynamoDBPersistenceLayer;
logger: Logger;
env: EnvVars;
sqsClient: SQSClient;
Expand All @@ -22,36 +24,35 @@ function createDocumentClient(): DynamoDBDocumentClient {
return DynamoDBDocumentClient.from(ddbClient);
}

function createSupplierConfigRepository(
log: Logger,
// eslint-disable-next-line @typescript-eslint/no-shadow
envVars: EnvVars,
): SupplierConfigRepository {
function createSupplierConfigRepository(): SupplierConfigRepository {
const config = {
supplierConfigTableName: envVars.SUPPLIER_CONFIG_TABLE_NAME,
};

return new SupplierConfigRepository(createDocumentClient(), config);
}

function createSupplierQuotasRepository(
log: Logger,
// eslint-disable-next-line @typescript-eslint/no-shadow
envVars: EnvVars,
): SupplierQuotasRepository {
function createSupplierQuotasRepository(): SupplierQuotasRepository {
const config = {
supplierQuotasTableName: envVars.SUPPLIER_QUOTAS_TABLE_NAME,
};

return new SupplierQuotasRepository(createDocumentClient(), config);
}

function createIdempotencyLayer(): DynamoDBPersistenceLayer {
return new DynamoDBPersistenceLayer({
tableName: envVars.IDEMPOTENCY_TABLE_NAME,
});
}

export function createDependenciesContainer(): Deps {
const log = createLogger({ logLevel: envVars.PINO_LOG_LEVEL });

return {
supplierConfigRepo: createSupplierConfigRepository(log, envVars),
supplierQuotasRepo: createSupplierQuotasRepository(log, envVars),
supplierConfigRepo: createSupplierConfigRepository(),
supplierQuotasRepo: createSupplierQuotasRepository(),
idempotencyLayer: createIdempotencyLayer(),
logger: log,
env: envVars,
sqsClient: new SQSClient({}),
Expand Down
1 change: 1 addition & 0 deletions lambdas/supplier-allocator/src/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const EnvVarsSchema = z.object({
SUPPLIER_CONFIG_TABLE_NAME: z.string(),
SUPPLIER_QUOTAS_TABLE_NAME: z.string(),
PINO_LOG_LEVEL: z.coerce.string().optional(),
IDEMPOTENCY_TABLE_NAME: z.string(),
});

export type EnvVars = z.infer<typeof EnvVarsSchema>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,11 @@ import {
$LetterStatusChangeEvent,
LetterStatusChangeEvent,
} from "@nhsdigital/nhs-notify-event-schemas-supplier-api/src/events/letter-events";
import {
SupplierConfigRepository,
SupplierQuotasRepository,
} from "@internal/datastore";
import { makeIdempotent } from "@aws-lambda-powertools/idempotency";
import createSupplierAllocatorHandler from "../allocate-handler";
import * as supplierConfig from "../../services/supplier-config";
import * as supplierQuotas from "../../services/supplier-quotas";
import * as allocationConfig from "../allocation-config";

import { Deps } from "../../config/deps";
import packageJson from "../../../package.json";

Expand All @@ -28,6 +24,14 @@ jest.mock("../../services/supplier-config");
jest.mock("../../services/supplier-quotas");
jest.mock("../allocation-config");

jest.mock("@aws-lambda-powertools/idempotency", () => {
const original = jest.requireActual("@aws-lambda-powertools/idempotency");
return {
...original,
makeIdempotent: jest.fn((fn, _) => fn),
};
});

function createSQSEvent(records: SQSRecord[]): SQSEvent {
return {
Records: records,
Expand Down Expand Up @@ -185,16 +189,15 @@ function setupDefaultMocks() {
}

describe("createSupplierAllocatorHandler", () => {
let mockSqsClient: jest.Mocked<SQSClient>;
let mockedDeps: jest.Mocked<Deps>;
let mockedSupplierConfigRepo: jest.Mocked<SupplierConfigRepository>;
let mockedSupplierQuotasRepo: jest.Mocked<SupplierQuotasRepository>;
beforeEach(() => {
mockSqsClient = {
send: jest.fn(),
} as unknown as jest.Mocked<SQSClient>;

mockedSupplierConfigRepo = {
const mockedDeps: jest.Mocked<Deps> = {
logger: { error: jest.fn(), info: jest.fn() } as unknown as pino.Logger,
env: {
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
IDEMPOTENCY_TABLE_NAME: "IdempotencyTable",
},
sqsClient: { send: jest.fn() } as unknown as SQSClient,
supplierConfigRepo: {
ddbClient: {} as any,
config: {} as any,
getLetterVariant: jest.fn(),
Expand All @@ -203,27 +206,18 @@ describe("createSupplierAllocatorHandler", () => {
getSuppliersDetails: jest.fn(),
getSupplierPacksForPackSpecification: jest.fn(),
getPackSpecification: jest.fn(),
};

mockedSupplierQuotasRepo = {
},
supplierQuotasRepo: {
ddbClient: {} as any,
config: {} as any,
getOverallAllocation: jest.fn(),
updateOverallAllocation: jest.fn(),
getDailyAllocation: jest.fn(),
updateDailyAllocation: jest.fn(),
};
},
} as unknown as Deps;

mockedDeps = {
logger: { error: jest.fn(), info: jest.fn() } as unknown as pino.Logger,
env: {
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
},
sqsClient: mockSqsClient,
supplierConfigRepo: mockedSupplierConfigRepo,
supplierQuotasRepo: mockedSupplierQuotasRepo,
};
beforeEach(() => {
jest.clearAllMocks();
});

Expand All @@ -244,8 +238,8 @@ describe("createSupplierAllocatorHandler", () => {

expect(result.batchItemFailures).toHaveLength(0);

expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
expect(sendCall).toBeInstanceOf(SendMessageCommand);

const messageBody = JSON.parse(sendCall.input.MessageBody);
Expand Down Expand Up @@ -281,8 +275,8 @@ describe("createSupplierAllocatorHandler", () => {

expect(result.batchItemFailures).toHaveLength(0);

expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
expect(sendCall).toBeInstanceOf(SendMessageCommand);

const messageBody = JSON.parse(sendCall.input.MessageBody);
Expand Down Expand Up @@ -315,8 +309,8 @@ describe("createSupplierAllocatorHandler", () => {

expect(result.batchItemFailures).toHaveLength(0);

expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
const messageBody = JSON.parse(sendCall.input.MessageBody);
expect(messageBody.allocationDetails.supplierSpec).toEqual({
supplierId: "supplier1",
Expand Down Expand Up @@ -361,7 +355,7 @@ describe("createSupplierAllocatorHandler", () => {
const handler = createSupplierAllocatorHandler(mockedDeps);
await handler(evt, {} as any, {} as any);

const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
const messageBody = JSON.parse(sendCall.input.MessageBody);
expect(messageBody.letterEvent.data.domainId).toBe("letter-test");
});
Expand Down Expand Up @@ -410,7 +404,7 @@ describe("createSupplierAllocatorHandler", () => {
if (!result) throw new Error("expected BatchResponse, got void");

expect(result.batchItemFailures).toHaveLength(0);
expect(mockSqsClient.send).toHaveBeenCalledTimes(2);
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(2);
});

test("returns batch failure for invalid JSON", async () => {
Expand Down Expand Up @@ -480,7 +474,7 @@ describe("createSupplierAllocatorHandler", () => {
process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue";

const sqsError = new Error("SQS send failed");
(mockSqsClient.send as jest.Mock).mockRejectedValueOnce(sqsError);
(mockedDeps.sqsClient.send as jest.Mock).mockRejectedValueOnce(sqsError);

const handler = createSupplierAllocatorHandler(mockedDeps);
const result = await handler(evt, {} as any, {} as any);
Expand Down Expand Up @@ -513,7 +507,7 @@ describe("createSupplierAllocatorHandler", () => {
expect(result.batchItemFailures).toHaveLength(1);
expect(result.batchItemFailures[0].itemIdentifier).toBe("fail-msg");

expect(mockSqsClient.send).toHaveBeenCalledTimes(2);
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(2);
});

test("sends correct queue URL in SQS message command", async () => {
Expand All @@ -529,7 +523,7 @@ describe("createSupplierAllocatorHandler", () => {
const handler = createSupplierAllocatorHandler(mockedDeps);
await handler(evt, {} as any, {} as any);

const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
expect(sendCall.input.QueueUrl).toBe(queueUrl);
});

Expand Down Expand Up @@ -557,8 +551,8 @@ describe("createSupplierAllocatorHandler", () => {
variantId: "lv1",
}),
);
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
expect(sendCall).toBeInstanceOf(SendMessageCommand);

const messageBody = JSON.parse(sendCall.input.MessageBody);
Expand Down Expand Up @@ -667,8 +661,9 @@ describe("createSupplierAllocatorHandler", () => {
variantId: "lv1",
}),
);
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock
.calls[0][0];
expect(sendCall).toBeInstanceOf(SendMessageCommand);

const messageBody = JSON.parse(sendCall.input.MessageBody);
Expand Down Expand Up @@ -711,8 +706,8 @@ describe("createSupplierAllocatorHandler", () => {
variantId: "lv1",
}),
);
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
expect(sendCall).toBeInstanceOf(SendMessageCommand);

const messageBody = JSON.parse(sendCall.input.MessageBody);
Expand Down Expand Up @@ -771,4 +766,22 @@ describe("createSupplierAllocatorHandler", () => {
expect(result.batchItemFailures).toHaveLength(0);
expect(allocationConfig.selectSupplierByFactor).toHaveBeenCalledTimes(2);
});

test("does not process a message more than once due to idempotency wrapper", async () => {
const preparedEvent = createPreparedV2Event();
const evt: SQSEvent = createSQSEvent([
createSqsRecord("msg1", JSON.stringify(preparedEvent)),
]);

process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue";

setupDefaultMocks();
(makeIdempotent as jest.Mock).mockImplementationOnce((_fn) => "supplier1");

const handler = createSupplierAllocatorHandler(mockedDeps);
await handler(evt, {} as any, {} as any);

expect(makeIdempotent).toHaveBeenCalledTimes(1);
expect(mockedDeps.sqsClient.send).not.toHaveBeenCalled();
});
});
Loading
Loading