Skip to content

Commit 4f8947f

Browse files
nhsd-david-wassnhsd-david-wass
committed
CCM-12392: Allocation idempotency
1 parent 9e46494 commit 4f8947f

17 files changed

Lines changed: 540 additions & 156 deletions

File tree

‎infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ module "supplier_allocator" {
3535
log_subscription_role_arn = local.acct.log_subscription_role_arn
3636

3737
lambda_env_vars = merge(local.common_lambda_env_vars, {
38-
UPSERT_LETTERS_QUEUE_URL = module.sqs_letter_updates.sqs_queue_url
38+
UPSERT_LETTERS_QUEUE_URL = module.sqs_letter_updates.sqs_queue_url,
39+
IDEMPOTENCY_TABLE_NAME = aws_dynamodb_table.idempotency.name
3940
})
4041
}
4142

@@ -110,6 +111,7 @@ data "aws_iam_policy_document" "supplier_allocator_lambda" {
110111

111112
resources = [
112113
aws_dynamodb_table.supplier-quotas.arn,
114+
aws_dynamodb_table.idempotency.arn,
113115
"${aws_dynamodb_table.supplier-quotas.arn}/index/*"
114116
]
115117
}

‎lambdas/supplier-allocator/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"dependencies": {
3+
"@aws-lambda-powertools/idempotency": "^2.33.0",
34
"@aws-sdk/client-dynamodb": "^3.858.0",
45
"@aws-sdk/client-sqs": "^3.984.0",
56
"@aws-sdk/lib-dynamodb": "^3.1044.0",

‎lambdas/supplier-allocator/src/config/__tests__/deps.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ describe("createDependenciesContainer", () => {
44
const env = {
55
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
66
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
7+
IDEMPOTENCY_TABLE_NAME: "IdempotencyTable",
78
};
89

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

31+
jest.mock("@aws-lambda-powertools/idempotency/dynamodb", () => ({
32+
DynamoDBPersistenceLayer: jest.fn(),
33+
}));
34+
3035
// Env
3136
jest.mock("../env", () => ({ envVars: env }));
3237
});
@@ -40,6 +45,9 @@ describe("createDependenciesContainer", () => {
4045
const { SupplierQuotasRepository } = jest.requireMock(
4146
"@internal/datastore",
4247
);
48+
const { DynamoDBPersistenceLayer } = jest.requireMock(
49+
"@aws-lambda-powertools/idempotency/dynamodb",
50+
);
4351
// eslint-disable-next-line @typescript-eslint/no-require-imports
4452
const { createDependenciesContainer } = require("../deps");
4553
const deps: Deps = createDependenciesContainer();
@@ -54,6 +62,11 @@ describe("createDependenciesContainer", () => {
5462
expect(supplierQuotasRepoCtorArgs[1]).toEqual({
5563
supplierQuotasTableName: "SupplierQuotasTable",
5664
});
65+
expect(DynamoDBPersistenceLayer).toHaveBeenCalledTimes(1);
66+
const idempotencyLayerCtorArgs = DynamoDBPersistenceLayer.mock.calls[0][0];
67+
expect(idempotencyLayerCtorArgs).toEqual({
68+
tableName: "IdempotencyTable",
69+
});
5770
expect(deps.env).toEqual(env);
5871
});
5972
});

‎lambdas/supplier-allocator/src/config/__tests__/env.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ describe("lambdaEnv", () => {
1616
it("should load all environment variables successfully", () => {
1717
process.env.SUPPLIER_CONFIG_TABLE_NAME = "SupplierConfigTable";
1818
process.env.SUPPLIER_QUOTAS_TABLE_NAME = "SupplierQuotasTable";
19+
process.env.IDEMPOTENCY_TABLE_NAME = "IdempotencyTable";
1920

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

2223
expect(envVars).toEqual({
2324
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
2425
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
26+
IDEMPOTENCY_TABLE_NAME: "IdempotencyTable",
2527
});
2628
});
2729
});

‎lambdas/supplier-allocator/src/config/deps.ts‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
22
import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb";
3+
import { DynamoDBPersistenceLayer } from "@aws-lambda-powertools/idempotency/dynamodb";
34
import { SQSClient } from "@aws-sdk/client-sqs";
45
import { Logger } from "pino";
56
import { createLogger } from "@internal/helpers";
@@ -12,6 +13,7 @@ import { EnvVars, envVars } from "./env";
1213
export type Deps = {
1314
supplierConfigRepo: SupplierConfigRepository;
1415
supplierQuotasRepo: SupplierQuotasRepository;
16+
idempotencyLayer: DynamoDBPersistenceLayer;
1517
logger: Logger;
1618
env: EnvVars;
1719
sqsClient: SQSClient;
@@ -22,36 +24,35 @@ function createDocumentClient(): DynamoDBDocumentClient {
2224
return DynamoDBDocumentClient.from(ddbClient);
2325
}
2426

25-
function createSupplierConfigRepository(
26-
log: Logger,
27-
// eslint-disable-next-line @typescript-eslint/no-shadow
28-
envVars: EnvVars,
29-
): SupplierConfigRepository {
27+
function createSupplierConfigRepository(): SupplierConfigRepository {
3028
const config = {
3129
supplierConfigTableName: envVars.SUPPLIER_CONFIG_TABLE_NAME,
3230
};
3331

3432
return new SupplierConfigRepository(createDocumentClient(), config);
3533
}
3634

37-
function createSupplierQuotasRepository(
38-
log: Logger,
39-
// eslint-disable-next-line @typescript-eslint/no-shadow
40-
envVars: EnvVars,
41-
): SupplierQuotasRepository {
35+
function createSupplierQuotasRepository(): SupplierQuotasRepository {
4236
const config = {
4337
supplierQuotasTableName: envVars.SUPPLIER_QUOTAS_TABLE_NAME,
4438
};
4539

4640
return new SupplierQuotasRepository(createDocumentClient(), config);
4741
}
4842

43+
function createIdempotencyLayer(): DynamoDBPersistenceLayer {
44+
return new DynamoDBPersistenceLayer({
45+
tableName: envVars.IDEMPOTENCY_TABLE_NAME,
46+
});
47+
}
48+
4949
export function createDependenciesContainer(): Deps {
5050
const log = createLogger({ logLevel: envVars.PINO_LOG_LEVEL });
5151

5252
return {
53-
supplierConfigRepo: createSupplierConfigRepository(log, envVars),
54-
supplierQuotasRepo: createSupplierQuotasRepository(log, envVars),
53+
supplierConfigRepo: createSupplierConfigRepository(),
54+
supplierQuotasRepo: createSupplierQuotasRepository(),
55+
idempotencyLayer: createIdempotencyLayer(),
5556
logger: log,
5657
env: envVars,
5758
sqsClient: new SQSClient({}),

‎lambdas/supplier-allocator/src/config/env.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const EnvVarsSchema = z.object({
44
SUPPLIER_CONFIG_TABLE_NAME: z.string(),
55
SUPPLIER_QUOTAS_TABLE_NAME: z.string(),
66
PINO_LOG_LEVEL: z.coerce.string().optional(),
7+
IDEMPOTENCY_TABLE_NAME: z.string(),
78
});
89

910
export type EnvVars = z.infer<typeof EnvVarsSchema>;

‎lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts‎

Lines changed: 73 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ import {
77
$LetterStatusChangeEvent,
88
LetterStatusChangeEvent,
99
} from "@nhsdigital/nhs-notify-event-schemas-supplier-api/src/events/letter-events";
10+
import { makeIdempotent } from "@aws-lambda-powertools/idempotency";
1011
import createSupplierAllocatorHandler from "../allocate-handler";
1112
import * as supplierConfig from "../../services/supplier-config";
1213
import * as supplierQuotas from "../../services/supplier-quotas";
1314
import * as allocationConfig from "../allocation-config";
14-
1515
import { Deps } from "../../config/deps";
1616
import packageJson from "../../../package.json";
1717

@@ -24,6 +24,14 @@ jest.mock("../../services/supplier-config");
2424
jest.mock("../../services/supplier-quotas");
2525
jest.mock("../allocation-config");
2626

27+
jest.mock("@aws-lambda-powertools/idempotency", () => {
28+
const original = jest.requireActual("@aws-lambda-powertools/idempotency");
29+
return {
30+
...original,
31+
makeIdempotent: jest.fn((fn, _) => fn),
32+
};
33+
});
34+
2735
function createSQSEvent(records: SQSRecord[]): SQSEvent {
2836
return {
2937
Records: records,
@@ -181,23 +189,35 @@ function setupDefaultMocks() {
181189
}
182190

183191
describe("createSupplierAllocatorHandler", () => {
184-
let mockSqsClient: jest.Mocked<SQSClient>;
185-
let mockedDeps: jest.Mocked<Deps>;
192+
const mockedDeps: jest.Mocked<Deps> = {
193+
logger: { error: jest.fn(), info: jest.fn() } as unknown as pino.Logger,
194+
env: {
195+
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
196+
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
197+
IDEMPOTENCY_TABLE_NAME: "IdempotencyTable",
198+
},
199+
sqsClient: { send: jest.fn() } as unknown as SQSClient,
200+
supplierConfigRepo: {
201+
ddbClient: {} as any,
202+
config: {} as any,
203+
getLetterVariant: jest.fn(),
204+
getVolumeGroup: jest.fn(),
205+
getSupplierAllocationsForVolumeGroup: jest.fn(),
206+
getSuppliersDetails: jest.fn(),
207+
getSupplierPacksForPackSpecification: jest.fn(),
208+
getPackSpecification: jest.fn(),
209+
},
210+
supplierQuotasRepo: {
211+
ddbClient: {} as any,
212+
config: {} as any,
213+
getOverallAllocation: jest.fn(),
214+
updateOverallAllocation: jest.fn(),
215+
getDailyAllocation: jest.fn(),
216+
updateDailyAllocation: jest.fn(),
217+
},
218+
} as unknown as Deps;
219+
186220
beforeEach(() => {
187-
mockSqsClient = {
188-
send: jest.fn(),
189-
} as unknown as jest.Mocked<SQSClient>;
190-
191-
mockedDeps = {
192-
logger: { error: jest.fn(), info: jest.fn() } as unknown as pino.Logger,
193-
env: {
194-
SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable",
195-
SUPPLIER_QUOTAS_TABLE_NAME: "SupplierQuotasTable",
196-
},
197-
sqsClient: mockSqsClient,
198-
supplierConfigRepo: {} as unknown as Deps["supplierConfigRepo"],
199-
supplierQuotasRepo: {} as unknown as Deps["supplierQuotasRepo"],
200-
};
201221
jest.clearAllMocks();
202222
});
203223

@@ -218,8 +238,8 @@ describe("createSupplierAllocatorHandler", () => {
218238

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

221-
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
222-
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
241+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
242+
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
223243
expect(sendCall).toBeInstanceOf(SendMessageCommand);
224244

225245
const messageBody = JSON.parse(sendCall.input.MessageBody);
@@ -255,8 +275,8 @@ describe("createSupplierAllocatorHandler", () => {
255275

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

258-
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
259-
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
278+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
279+
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
260280
expect(sendCall).toBeInstanceOf(SendMessageCommand);
261281

262282
const messageBody = JSON.parse(sendCall.input.MessageBody);
@@ -289,8 +309,8 @@ describe("createSupplierAllocatorHandler", () => {
289309

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

292-
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
293-
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
312+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
313+
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
294314
const messageBody = JSON.parse(sendCall.input.MessageBody);
295315
expect(messageBody.allocationDetails.supplierSpec).toEqual({
296316
supplierId: "supplier1",
@@ -335,7 +355,7 @@ describe("createSupplierAllocatorHandler", () => {
335355
const handler = createSupplierAllocatorHandler(mockedDeps);
336356
await handler(evt, {} as any, {} as any);
337357

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

386406
expect(result.batchItemFailures).toHaveLength(0);
387-
expect(mockSqsClient.send).toHaveBeenCalledTimes(2);
407+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(2);
388408
});
389409

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

456476
const sqsError = new Error("SQS send failed");
457-
(mockSqsClient.send as jest.Mock).mockRejectedValueOnce(sqsError);
477+
(mockedDeps.sqsClient.send as jest.Mock).mockRejectedValueOnce(sqsError);
458478

459479
const handler = createSupplierAllocatorHandler(mockedDeps);
460480
const result = await handler(evt, {} as any, {} as any);
@@ -487,7 +507,7 @@ describe("createSupplierAllocatorHandler", () => {
487507
expect(result.batchItemFailures).toHaveLength(1);
488508
expect(result.batchItemFailures[0].itemIdentifier).toBe("fail-msg");
489509

490-
expect(mockSqsClient.send).toHaveBeenCalledTimes(2);
510+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(2);
491511
});
492512

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

506-
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
526+
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
507527
expect(sendCall.input.QueueUrl).toBe(queueUrl);
508528
});
509529

@@ -531,8 +551,8 @@ describe("createSupplierAllocatorHandler", () => {
531551
variantId: "lv1",
532552
}),
533553
);
534-
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
535-
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
554+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
555+
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
536556
expect(sendCall).toBeInstanceOf(SendMessageCommand);
537557

538558
const messageBody = JSON.parse(sendCall.input.MessageBody);
@@ -641,8 +661,9 @@ describe("createSupplierAllocatorHandler", () => {
641661
variantId: "lv1",
642662
}),
643663
);
644-
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
645-
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
664+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
665+
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock
666+
.calls[0][0];
646667
expect(sendCall).toBeInstanceOf(SendMessageCommand);
647668

648669
const messageBody = JSON.parse(sendCall.input.MessageBody);
@@ -685,8 +706,8 @@ describe("createSupplierAllocatorHandler", () => {
685706
variantId: "lv1",
686707
}),
687708
);
688-
expect(mockSqsClient.send).toHaveBeenCalledTimes(1);
689-
const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0];
709+
expect(mockedDeps.sqsClient.send).toHaveBeenCalledTimes(1);
710+
const sendCall = (mockedDeps.sqsClient.send as jest.Mock).mock.calls[0][0];
690711
expect(sendCall).toBeInstanceOf(SendMessageCommand);
691712

692713
const messageBody = JSON.parse(sendCall.input.MessageBody);
@@ -745,4 +766,22 @@ describe("createSupplierAllocatorHandler", () => {
745766
expect(result.batchItemFailures).toHaveLength(0);
746767
expect(allocationConfig.selectSupplierByFactor).toHaveBeenCalledTimes(2);
747768
});
769+
770+
test("does not process a message more than once due to idempotency wrapper", async () => {
771+
const preparedEvent = createPreparedV2Event();
772+
const evt: SQSEvent = createSQSEvent([
773+
createSqsRecord("msg1", JSON.stringify(preparedEvent)),
774+
]);
775+
776+
process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue";
777+
778+
setupDefaultMocks();
779+
(makeIdempotent as jest.Mock).mockImplementationOnce((_fn) => "supplier1");
780+
781+
const handler = createSupplierAllocatorHandler(mockedDeps);
782+
await handler(evt, {} as any, {} as any);
783+
784+
expect(makeIdempotent).toHaveBeenCalledTimes(1);
785+
expect(mockedDeps.sqsClient.send).not.toHaveBeenCalled();
786+
});
748787
});

0 commit comments

Comments
 (0)