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
1 change: 1 addition & 0 deletions infrastructure/terraform/components/api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ No requirements.
| <a name="module_amendment_event_transformer"></a> [amendment\_event\_transformer](#module\_amendment\_event\_transformer) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.29/terraform-lambda.zip | n/a |
| <a name="module_amendments_queue"></a> [amendments\_queue](#module\_amendments\_queue) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/3.0.6/terraform-sqs.zip | n/a |
| <a name="module_authorizer_lambda"></a> [authorizer\_lambda](#module\_authorizer\_lambda) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.29/terraform-lambda.zip | n/a |
| <a name="module_ddb_alarms_idempotency"></a> [ddb\_alarms\_idempotency](#module\_ddb\_alarms\_idempotency) | ../../modules/alarms-ddb | n/a |
| <a name="module_ddb_alarms_letter_queue"></a> [ddb\_alarms\_letter\_queue](#module\_ddb\_alarms\_letter\_queue) | ../../modules/alarms-ddb | n/a |
| <a name="module_ddb_alarms_letters"></a> [ddb\_alarms\_letters](#module\_ddb\_alarms\_letters) | ../../modules/alarms-ddb | n/a |
| <a name="module_ddb_alarms_mi"></a> [ddb\_alarms\_mi](#module\_ddb\_alarms\_mi) | ../../modules/alarms-ddb | n/a |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
resource "aws_dynamodb_table" "idempotency" {
name = "${local.csi}-idempotency"
billing_mode = "PAY_PER_REQUEST"
hash_key = "id"
attribute {
name = "id"
type = "S"
}
ttl {
attribute_name = "expiration"
enabled = true
}
Comment thread
stevebux marked this conversation as resolved.

point_in_time_recovery {
enabled = true
}

tags = merge(
local.default_tags,
{
NHSE-Enable-Dynamo-Backup-Acct = "True"
}
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module "ddb_alarms_idempotency" {
count = local.alarms_enabled ? 1 : 0
source = "../../modules/alarms-ddb"
alarm_prefix = local.csi
table_name = aws_dynamodb_table.idempotency.name
tags = local.default_tags
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ module "upsert_letter" {
log_subscription_role_arn = local.acct.log_subscription_role_arn

lambda_env_vars = merge(local.common_lambda_env_vars, {
VARIANT_MAP = jsonencode(var.letter_variant_map)
VARIANT_MAP = jsonencode(var.letter_variant_map),
IDEMPOTENCY_TABLE_NAME = aws_dynamodb_table.idempotency.name
})
}

Expand Down Expand Up @@ -67,6 +68,7 @@ data "aws_iam_policy_document" "upsert_letter_lambda" {

resources = [
aws_dynamodb_table.letters.arn,
aws_dynamodb_table.idempotency.arn,
"${aws_dynamodb_table.letters.arn}/index/supplierStatus-index"
]
}
Expand Down
1 change: 1 addition & 0 deletions lambdas/upsert-letter/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.984.0",
"@aws-sdk/lib-dynamodb": "^3.1008.0",
"@internal/datastore": "*",
Expand Down
2 changes: 2 additions & 0 deletions lambdas/upsert-letter/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.LETTERS_TABLE_NAME = "letters-table";
process.env.IDEMPOTENCY_TABLE_NAME = "idempotency-table";
process.env.LETTER_TTL_HOURS = "12960";

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

expect(envVars).toEqual({
LETTERS_TABLE_NAME: "letters-table",
IDEMPOTENCY_TABLE_NAME: "idempotency-table",
LETTER_TTL_HOURS: 12_960,
});
});
Expand Down
17 changes: 11 additions & 6 deletions lambdas/upsert-letter/src/config/deps.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb";
import { DynamoDBPersistenceLayer } from "@aws-lambda-powertools/idempotency/dynamodb";
import { Logger } from "pino";
import { LetterRepository } from "@internal/datastore";
import { createLogger } from "@internal/helpers";
import { EnvVars, envVars } from "./env";

export type Deps = {
letterRepo: LetterRepository;
idempotencyLayer: DynamoDBPersistenceLayer;
logger: Logger;
env: EnvVars;
};
Expand All @@ -16,11 +18,7 @@ function createDocumentClient(): DynamoDBDocumentClient {
return DynamoDBDocumentClient.from(ddbClient);
}

function createLetterRepository(
log: Logger,
// eslint-disable-next-line @typescript-eslint/no-shadow
envVars: EnvVars,
): LetterRepository {
function createLetterRepository(log: Logger): LetterRepository {
const config = {
lettersTableName: envVars.LETTERS_TABLE_NAME,
lettersTtlHours: envVars.LETTER_TTL_HOURS,
Expand All @@ -29,11 +27,18 @@ function createLetterRepository(
return new LetterRepository(createDocumentClient(), log, 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 {
letterRepo: createLetterRepository(log, envVars),
letterRepo: createLetterRepository(log),
idempotencyLayer: createIdempotencyLayer(),
logger: log,
env: envVars,
};
Expand Down
1 change: 1 addition & 0 deletions lambdas/upsert-letter/src/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const EnvVarsSchema = z.object({
LETTERS_TABLE_NAME: z.string(),
LETTER_TTL_HOURS: z.coerce.number().int(),
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 @@ -6,6 +6,7 @@ import {
} from "@internal/datastore";
import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering";
import { LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1";
import { makeIdempotent } from "@aws-lambda-powertools/idempotency";
import {
$LetterStatusChangeEvent,
LetterStatusChangeEvent,
Expand All @@ -15,6 +16,14 @@ import { Deps } from "../../config/deps";
import { EnvVars } from "../../config/env";
import packageJson from "../../../package.json";

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

const renderingSchemaVersion: string =
packageJson.dependencies[
"@nhsdigital/nhs-notify-event-schemas-letter-rendering"
Expand Down Expand Up @@ -313,7 +322,7 @@ describe("createUpsertLetterHandler", () => {
);
});

it("does not treat a replayed insert as a failure", async () => {
it("does not treat a second insert for the same letter as a failure", async () => {
const v1message = {
letterEvent: createPreparedV1Event(),
supplierSpec: {
Expand All @@ -338,6 +347,26 @@ describe("createUpsertLetterHandler", () => {
expect(result!.batchItemFailures).toEqual([]);
});

it("does not insert a letter if the same message is replayed", async () => {
const v1message = {
letterEvent: createPreparedV1Event(),
supplierSpec: {
supplierId: "supplier1",
specId: "spec1",
priority: 10,
billingId: "billing1",
},
};
const evt: SQSEvent = createSQSEvent([
createSqsRecord("msg2", JSON.stringify(v1message)),
]);
(makeIdempotent as jest.Mock).mockImplementationOnce((_fn) => "supplier1");

await createUpsertLetterHandler(mockedDeps)(evt, {} as any, {} as any);

expect(mockedDeps.letterRepo.putLetter).not.toHaveBeenCalled();
});
Comment thread
stevebux marked this conversation as resolved.

test("unknown supplier has metric emitted with 'unknown' supplier dimension", async () => {
const letterEvent = createSupplierStatusChangeEventWithoutSupplier();

Expand Down
70 changes: 48 additions & 22 deletions lambdas/upsert-letter/src/handler/upsert-handler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SQSBatchItemFailure, SQSEvent, SQSHandler } from "aws-lambda";
import { Context, SQSBatchItemFailure, SQSEvent, SQSHandler } from "aws-lambda";
import { $LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1";
import {
InsertLetter,
Expand All @@ -11,6 +11,10 @@ import {
} from "@nhsdigital/nhs-notify-event-schemas-supplier-api/src/events/letter-events";
import { $LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering";
import { MetricsLogger, Unit, metricScope } from "aws-embedded-metrics";
import {
IdempotencyConfig,
makeIdempotent,
} from "@aws-lambda-powertools/idempotency";
import { Deps } from "../config/deps";
Comment thread
stevebux marked this conversation as resolved.
import {
PreparedEvents,
Expand All @@ -20,6 +24,10 @@ import {
UpsertOperation,
} from "./schemas";

const idempotencyConfig = new IdempotencyConfig({
eventKeyJmesPath: "id",
});

function getOperationFromType(type: string): UpsertOperation {
if (
type.startsWith("uk.nhs.notify.letter-rendering.letter-request.prepared")
Expand Down Expand Up @@ -180,8 +188,13 @@ function parseQueueMessage(queueMessage: string): QueueMessage {
}

export default function createUpsertLetterHandler(deps: Deps): SQSHandler {
const processRecordIdempotently = makeIdempotent(processRecord, {
persistenceStore: deps.idempotencyLayer,
config: idempotencyConfig,
});

return metricScope((metrics: MetricsLogger) => {
return async (event: SQSEvent) => {
return async (event: SQSEvent, context: Context) => {
const batchItemFailures: SQSBatchItemFailure[] = [];
const perSupplierSuccess: Map<string, number> = new Map<string, number>();
const perSupplierFailure: Map<string, number> = new Map<string, number>();
Expand Down Expand Up @@ -216,29 +229,13 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler {
supplier: supplierSpec,
});

supplier =
!supplierSpec || !supplierSpec.supplierId
? getSupplierIdFromEvent(letterEvent)
: supplierSpec.supplierId;

const operation = getOperationFromType(letterEvent.type);

await runUpsert(
operation,
idempotencyConfig.registerLambdaContext(context);
supplier = await processRecordIdempotently(
letterEvent,
supplierSpec ?? {
supplierId: "unknown",
specId: "unknown",
priority: 10,
billingId: "unknown",
},
supplierSpec,
perSupplierSuccess,
deps,
);
Comment thread
stevebux marked this conversation as resolved.
Comment thread
stevebux marked this conversation as resolved.

perSupplierSuccess.set(
supplier,
(perSupplierSuccess.get(supplier) || 0) + 1,
);
} catch (error) {
deps.logger.error({
description: "Error processing upsert of record",
Expand All @@ -261,3 +258,32 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler {
};
});
}

async function processRecord(
letterEvent: LetterStatusChangeEvent | PreparedEvents,
supplierSpec: SupplierSpec | undefined,
perSupplierSuccess: Map<string, number>,
deps: Deps,
) {
const supplier =
!supplierSpec || !supplierSpec.supplierId
? getSupplierIdFromEvent(letterEvent)
: supplierSpec.supplierId;

const operation = getOperationFromType(letterEvent.type);

await runUpsert(
operation,
letterEvent,
supplierSpec ?? {
supplierId: "unknown",
specId: "unknown",
priority: 10,
billingId: "unknown",
},
deps,
);

perSupplierSuccess.set(supplier, (perSupplierSuccess.get(supplier) || 0) + 1);
return supplier;
}
59 changes: 56 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading