Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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