From 365c222cf32af517e8ec9516dc507388d3c479e6 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Mon, 13 Apr 2026 15:22:42 +0100 Subject: [PATCH 01/16] [HOTE-1100] feat: result-status-lambda --- lambdas/src/lib/db/order-db.test.ts | 59 +++- lambdas/src/lib/db/order-db.ts | 30 +- lambdas/src/lib/db/result-db.test.ts | 55 ++++ lambdas/src/lib/db/result-db.ts | 34 ++ .../src/result-status-lambda/index.test.ts | 294 ++++++++++++++++++ lambdas/src/result-status-lambda/index.ts | 143 +++++++++ lambdas/src/result-status-lambda/init.test.ts | 147 +++++++++ lambdas/src/result-status-lambda/init.ts | 34 ++ lambdas/src/result-status-lambda/models.ts | 28 ++ 9 files changed, 819 insertions(+), 5 deletions(-) create mode 100644 lambdas/src/lib/db/result-db.test.ts create mode 100644 lambdas/src/lib/db/result-db.ts create mode 100644 lambdas/src/result-status-lambda/index.test.ts create mode 100644 lambdas/src/result-status-lambda/index.ts create mode 100644 lambdas/src/result-status-lambda/init.test.ts create mode 100644 lambdas/src/result-status-lambda/init.ts create mode 100644 lambdas/src/result-status-lambda/models.ts diff --git a/lambdas/src/lib/db/order-db.test.ts b/lambdas/src/lib/db/order-db.test.ts index 738eb095c..04cf47ed5 100644 --- a/lambdas/src/lib/db/order-db.test.ts +++ b/lambdas/src/lib/db/order-db.test.ts @@ -1,7 +1,6 @@ -import { OrderResultSummary, OrderService } from "./order-db"; -import { OrderStatus, ResultStatus } from "../types/status"; - import { Commons } from "../commons"; +import { OrderStatus, ResultStatus } from "../types/status"; +import { OrderResultSummary, OrderService } from "./order-db"; const normalizeWhitespace = (sql: string): string => sql.replace(/\s+/g, " ").trim(); @@ -94,6 +93,60 @@ describe("OrderService", () => { }); }); + describe("retrievePatientIdFromOrder", () => { + const expectedRetrievePatientIdQuery = ` + SELECT o.patient_uid, + o.order_uid + FROM test_order o + WHERE o.order_uid = $1::uuid + ORDER BY o.created_at DESC + LIMIT 1; + `; + + it("should return patient reference when found", async () => { + const mockReference = { + order_uid: "order-123", + patient_uid: "patient-abc", + }; + dbClient.query.mockResolvedValue({ rows: [mockReference] }); + + const result = await orderService.retrievePatientIdFromOrder("order-123"); + + expect(dbClient.query).toHaveBeenCalledTimes(1); + expect(normalizeWhitespace(dbClient.query.mock.calls[0][0])).toBe( + normalizeWhitespace(expectedRetrievePatientIdQuery), + ); + expect(dbClient.query.mock.calls[0][1]).toEqual(["order-123"]); + expect(result).toEqual(mockReference); + }); + + it("should return null when no order is found", async () => { + dbClient.query.mockResolvedValue({ rows: [] }); + + const result = await orderService.retrievePatientIdFromOrder("order-404"); + + expect(dbClient.query).toHaveBeenCalledTimes(1); + expect(normalizeWhitespace(dbClient.query.mock.calls[0][0])).toBe( + normalizeWhitespace(expectedRetrievePatientIdQuery), + ); + expect(dbClient.query.mock.calls[0][1]).toEqual(["order-404"]); + expect(result).toBeNull(); + }); + + it("should rethrow when retrieving patient reference fails", async () => { + const error = new Error("query failed"); + dbClient.query.mockRejectedValue(error); + + await expect(orderService.retrievePatientIdFromOrder("order-500")).rejects.toThrow(error); + + expect(dbClient.query).toHaveBeenCalledTimes(1); + expect(normalizeWhitespace(dbClient.query.mock.calls[0][0])).toBe( + normalizeWhitespace(expectedRetrievePatientIdQuery), + ); + expect(dbClient.query.mock.calls[0][1]).toEqual(["order-500"]); + }); + }); + describe("updateOrderStatusAndResultStatus", () => { it("should execute both SQL statements with the expected full queries and parameters", async () => { const tx = { diff --git a/lambdas/src/lib/db/order-db.ts b/lambdas/src/lib/db/order-db.ts index a002d0c35..67f784754 100644 --- a/lambdas/src/lib/db/order-db.ts +++ b/lambdas/src/lib/db/order-db.ts @@ -1,6 +1,5 @@ -import { OrderStatus, ResultStatus } from "../types/status"; - import { Commons } from "../commons"; +import { OrderStatus, ResultStatus } from "../types/status"; import { DBClient } from "./db-client"; export interface OrderResultSummary { @@ -12,6 +11,11 @@ export interface OrderResultSummary { order_status_code: OrderStatus | null; } +export interface OrderPatientReference { + order_uid: string; + patient_uid: string; +} + export class OrderService { private readonly dbClient: DBClient; private readonly commons: Commons; @@ -46,6 +50,28 @@ export class OrderService { } } + async retrievePatientIdFromOrder(orderUid: string): Promise { + const query = ` + SELECT o.patient_uid, + o.order_uid + FROM test_order o + WHERE o.order_uid = $1::uuid + ORDER BY o.created_at DESC + LIMIT 1; + `; + + try { + const result = await this.dbClient.query(query, [orderUid]); + return result.rows[0] || null; + } catch (error) { + this.commons.logError("order-db", "Failed to retrieve order-patient association", { + error, + orderUid, + }); + throw error; + } + } + async updateOrderStatusAndResultStatus( orderUid: string, statusCode: OrderStatus, diff --git a/lambdas/src/lib/db/result-db.test.ts b/lambdas/src/lib/db/result-db.test.ts new file mode 100644 index 000000000..6b39f151c --- /dev/null +++ b/lambdas/src/lib/db/result-db.test.ts @@ -0,0 +1,55 @@ +import { Commons } from "../commons"; +import { ResultStatus } from "../types/status"; +import { DBClient } from "./db-client"; +import { ResultService } from "./result-db"; + +const mockQuery = jest.fn(); + +const mockDbClient: DBClient = { + query: mockQuery, + close: jest.fn().mockResolvedValue(undefined), + withTransaction: jest.fn(), +}; + +const mockCommons: Commons = { + logError: jest.fn(), + logInfo: jest.fn(), + logDebug: jest.fn(), +}; + +const orderUid = "order-123"; +const correlationId = "corr-xyz"; + +describe("ResultService", () => { + let resultService: ResultService; + + beforeEach(() => { + jest.clearAllMocks(); + mockQuery.mockReset(); + resultService = new ResultService(mockDbClient, mockCommons); + }); + + describe("updateResultStatus", () => { + it("should execute the expected SQL statement with the correct parameters", async () => { + await resultService.updateResultStatus( + orderUid, + ResultStatus.Result_Available, + correlationId, + ); + + expect(mockDbClient.query).toHaveBeenCalledWith( + expect.stringContaining("INSERT INTO result_status"), + [orderUid, ResultStatus.Result_Available, correlationId], + ); + }); + + it("should rethrow if the database query fails", async () => { + const mockError = new Error("Database error"); + mockQuery.mockRejectedValue(mockError); + + await expect( + resultService.updateResultStatus(orderUid, ResultStatus.Result_Available, correlationId), + ).rejects.toThrow("Database error"); + }); + }); +}); diff --git a/lambdas/src/lib/db/result-db.ts b/lambdas/src/lib/db/result-db.ts new file mode 100644 index 000000000..0664363e8 --- /dev/null +++ b/lambdas/src/lib/db/result-db.ts @@ -0,0 +1,34 @@ +import { Commons } from "../commons"; +import { ResultStatus } from "../types/status"; +import { DBClient } from "./db-client"; + +export class ResultService { + private readonly dbClient: DBClient; + private readonly commons: Commons; + constructor(dbClient: DBClient, commons: Commons) { + this.dbClient = dbClient; + this.commons = commons; + } + + async updateResultStatus( + orderUid: string, + status: ResultStatus, + correlationId: string | null, + ): Promise { + const query = ` + INSERT INTO result_status (order_uid, status, correlation_id) + VALUES ($1::uuid, $2, $3) + `; + try { + await this.dbClient.query(query, [orderUid, status, correlationId]); + } catch (error) { + this.commons.logError("result-db", "Failed to update result status", { + error, + orderUid, + status, + correlationId, + }); + throw error; + } + } +} diff --git a/lambdas/src/result-status-lambda/index.test.ts b/lambdas/src/result-status-lambda/index.test.ts new file mode 100644 index 000000000..267626dbd --- /dev/null +++ b/lambdas/src/result-status-lambda/index.test.ts @@ -0,0 +1,294 @@ +import { APIGatewayProxyEvent } from "aws-lambda"; + +import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; +import { ResultStatus } from "../lib/types/status"; +import { handler } from "./index"; + +const mockLogInfo = jest.fn(); +const mockLogDebug = jest.fn(); +const mockLogError = jest.fn(); +const mockUpdateResultStatus = jest.fn(); +const mockRetrievePatientIdFromOrder = jest.fn(); + +jest.mock("./init", () => ({ + init: jest.fn(() => ({ + commons: { + logInfo: mockLogInfo, + logDebug: mockLogDebug, + logError: mockLogError, + }, + resultService: { + updateResultStatus: mockUpdateResultStatus, + }, + orderService: { + retrievePatientIdFromOrder: mockRetrievePatientIdFromOrder, + }, + })), +})); + +jest.mock("../lib/fhir-response", () => ({ + createFhirErrorResponse: jest.fn((code, type, message, severity) => ({ + statusCode: code, + body: JSON.stringify({ issue: [{ code: type, diagnostics: message, severity }] }), + })), + createFhirResponse: jest.fn((code, resource) => ({ + statusCode: code, + body: JSON.stringify(resource), + })), +})); + +const VALID_ORDER_UUID = "123a1234-a12b-1234-abcd-1234567890ab"; +const VALID_PATIENT_UUID = "123a1234-a12b-1234-abcd-1234567890ab"; +const VALID_CORRELATION_ID = "123a1234-a12b-1234-abcd-1234567890ab"; + +const validTask = { + resourceType: "Task", + status: "completed", + intent: "order", + identifier: [{ value: VALID_ORDER_UUID }], + for: { reference: `Patient/${VALID_PATIENT_UUID}` }, + businessStatus: { coding: [{ code: "result-available" }] }, + basedOn: [{ reference: "ServiceRequest/some-ref" }], +}; + +const makeEvent = ( + body: string | null, + headers: Record = {}, +): APIGatewayProxyEvent => + ({ + body, + headers, + }) as APIGatewayProxyEvent; + +const validEventHeaders = { "X-Correlation-ID": VALID_CORRELATION_ID }; + +describe("result-status-lambda handler", () => { + beforeEach(() => { + jest.clearAllMocks(); + + mockLogInfo.mockReset(); + mockLogDebug.mockReset(); + mockLogError.mockReset(); + mockUpdateResultStatus.mockReset(); + mockRetrievePatientIdFromOrder.mockReset(); + + mockRetrievePatientIdFromOrder.mockResolvedValue({ + order_uid: VALID_ORDER_UUID, + patient_uid: VALID_PATIENT_UUID, + }); + mockUpdateResultStatus.mockResolvedValue(undefined); + }); + + describe("request body parsing", () => { + it("returns 400 when body is null", async () => { + const res = await handler(makeEvent(null)); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + "Request body is required", + "error", + ); + }); + + it("returns 400 when body is invalid JSON", async () => { + const res = await handler(makeEvent("not-valid-json")); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + "Invalid JSON in request body", + "error", + ); + }); + + it("returns 400 when task fails schema validation", async () => { + const res = await handler(makeEvent(JSON.stringify({ resourceType: "Task" }))); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + expect.stringContaining("Task validation failed"), + "error", + ); + }); + + it("returns 400 when resourceType is not Task", async () => { + const invalidTask = { ...validTask, resourceType: "Observation" }; + + const res = await handler(makeEvent(JSON.stringify(invalidTask))); + + expect(res.statusCode).toBe(400); + }); + + it("returns 400 when businessStatus coding code is not result-available", async () => { + const invalidTask = { + ...validTask, + businessStatus: { coding: [{ code: ResultStatus.Result_Withheld }] }, + }; + + const res = await handler(makeEvent(JSON.stringify(invalidTask))); + + expect(res.statusCode).toBe(400); + }); + + it("returns 400 when identifier array is empty", async () => { + const invalidTask = { ...validTask, identifier: [] }; + + const res = await handler(makeEvent(JSON.stringify(invalidTask))); + + expect(res.statusCode).toBe(400); + }); + }); + + describe("FHIR Task identifier extraction", () => { + it("returns 400 when for.reference has no slash (single segment)", async () => { + const invalidTask = { ...validTask, for: { reference: VALID_PATIENT_UUID } }; + + const res = await handler(makeEvent(JSON.stringify(invalidTask))); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + "Invalid for.reference format", + "error", + ); + }); + + it("returns 400 when for.reference has more than two segments", async () => { + const invalidTask = { + ...validTask, + for: { reference: `Patient/${VALID_PATIENT_UUID}/extra` }, + }; + + const res = await handler(makeEvent(JSON.stringify(invalidTask))); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + "Invalid for.reference format", + "error", + ); + }); + + it("returns 400 when patient ID in for.reference is not a valid UUID", async () => { + const invalidTask = { ...validTask, for: { reference: "Patient/not-a-uuid" } }; + + const res = await handler(makeEvent(JSON.stringify(invalidTask))); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + "Invalid patient ID format", + "error", + ); + }); + + it("returns 400 when order identifier value is not a valid UUID", async () => { + const invalidTask = { ...validTask, identifier: [{ value: "not-a-uuid" }] }; + + const res = await handler(makeEvent(JSON.stringify(invalidTask))); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + "Invalid identifier.value format", + "error", + ); + }); + }); + + describe("order lookup", () => { + it("returns 500 when orderService.retrievePatientIdFromOrder throws", async () => { + mockRetrievePatientIdFromOrder.mockRejectedValueOnce(new Error("DB connection failed")); + + const res = await handler(makeEvent(JSON.stringify(validTask))); + + expect(res.statusCode).toBe(500); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 500, + "exception", + "An internal error occurred", + "fatal", + ); + }); + + it("returns 404 when order is not found", async () => { + mockRetrievePatientIdFromOrder.mockResolvedValueOnce(null); + + const res = await handler(makeEvent(JSON.stringify(validTask))); + + expect(res.statusCode).toBe(404); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 404, + "not_found", + "Order not found", + "error", + ); + }); + }); + + describe("patient authorisation", () => { + it("returns 403 when patient UID in task does not match order record", async () => { + mockRetrievePatientIdFromOrder.mockResolvedValueOnce({ + order_uid: VALID_ORDER_UUID, + patient_uid: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", + }); + + const res = await handler(makeEvent(JSON.stringify(validTask))); + + expect(res.statusCode).toBe(403); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 403, + "forbidden", + "Patient UID does not match order record", + "error", + ); + }); + }); + + describe("result status update", () => { + it("returns 500 when resultService.updateResultStatus throws", async () => { + mockUpdateResultStatus.mockRejectedValueOnce(new Error("DB write failed")); + + const res = await handler(makeEvent(JSON.stringify(validTask), validEventHeaders)); + + expect(res.statusCode).toBe(500); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 500, + "exception", + "An internal error occurred", + "fatal", + ); + }); + + it("calls updateResultStatus with correct arguments", async () => { + await handler(makeEvent(JSON.stringify(validTask), validEventHeaders)); + + expect(mockUpdateResultStatus).toHaveBeenCalledWith( + VALID_ORDER_UUID, + ResultStatus.Result_Available, + VALID_CORRELATION_ID, + ); + }); + }); + + describe("success", () => { + it("returns 201 with the original task on success", async () => { + const res = await handler(makeEvent(JSON.stringify(validTask), validEventHeaders)); + + expect(res.statusCode).toBe(201); + expect(createFhirResponse).toHaveBeenCalledWith( + 201, + expect.objectContaining({ resourceType: "Task" }), + ); + }); + }); +}); diff --git a/lambdas/src/result-status-lambda/index.ts b/lambdas/src/result-status-lambda/index.ts new file mode 100644 index 000000000..f3a62adfa --- /dev/null +++ b/lambdas/src/result-status-lambda/index.ts @@ -0,0 +1,143 @@ +import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; + +import { ConsoleCommons } from "../lib/commons"; +import { type OrderPatientReference } from "../lib/db/order-db"; +import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; +import { type FHIRTask } from "../lib/models/fhir/fhir-service-request-type"; +import { ResultStatus } from "../lib/types/status"; +import { getCorrelationIdFromEventHeaders, isUUID } from "../lib/utils/utils"; +import { generateReadableError } from "../lib/utils/validation-utils"; +import { init } from "./init"; +import { resultStatusFHIRTaskSchema } from "./models"; + +function parseAndValidateTask(body: string | null, commons: ConsoleCommons): FHIRTask { + let parsedTask: string; + + if (!body) { + commons.logError("result-status-lambda", "Missing request body"); + throw new Error("Request body is required"); + } + + try { + parsedTask = JSON.parse(body); + } catch (error) { + commons.logError("result-status-lambda", "Invalid JSON in request body", { error }); + throw new Error("Invalid JSON in request body", { cause: error }); + } + + const validationResult = resultStatusFHIRTaskSchema.safeParse(parsedTask); + + if (!validationResult.success) { + const errorDetails = generateReadableError(validationResult.error); + commons.logError("result-status-lambda", "Task validation failed", { error: errorDetails }); + throw new Error(`Task validation failed: ${errorDetails}`); + } + + return validationResult.data; +} + +function extractPatientIdFromFHIRTask(task: FHIRTask): string { + const parts = task.for.reference.split("/"); + if (parts.length !== 2) { + throw new Error("Invalid for.reference format"); + } + + const patientId = parts[1]; + + if (!isUUID(patientId)) { + throw new Error("Invalid patient ID format"); + } + + return patientId; +} + +function extractOrderUidFromFHIRTask(task: FHIRTask): string { + const orderUid = task.identifier?.[0]?.value; + + if (!orderUid) { + throw new Error("Missing identifier.value"); + } + if (!isUUID(orderUid)) { + throw new Error("Invalid identifier.value format"); + } + + return orderUid; +} + +/** + * Lambda handler for POST /result/status endpoint + * Accepts FHIR Task resources and updates result status on database after validation and business logic checks. + * Returns appropriate FHIR responses for success and error cases. + */ +export const handler = async (event: APIGatewayProxyEvent): Promise => { + const { commons, resultService, orderService } = init(); + commons.logInfo("result-status-lambda", "Received result status request", { + path: event.path, + method: event.httpMethod, + }); + + let task: FHIRTask; + + try { + task = parseAndValidateTask(event.body, commons); + } catch (error) { + const message = error instanceof Error ? error.message : "Invalid request body"; + return createFhirErrorResponse(400, "invalid", message, "error"); + } + + let patientUID: string, orderUID: string; + + try { + patientUID = extractPatientIdFromFHIRTask(task); + orderUID = extractOrderUidFromFHIRTask(task); + } catch (error) { + const message = error instanceof Error ? error.message : "Invalid identifiers"; + commons.logError("result-status-lambda", "Failed to extract identifiers from FHIR Task", { + error, + }); + return createFhirErrorResponse(400, "invalid", message, "error"); + } + + let orderSummary: OrderPatientReference | null; + + try { + orderSummary = await orderService.retrievePatientIdFromOrder(orderUID); + } catch (error) { + commons.logError("result-status-lambda", "Failed to retrieve order details from database", { + error, + orderUID, + }); + return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); + } + + if (!orderSummary) { + commons.logError("result-status-lambda", "Order not found for given order UID", { orderUID }); + return createFhirErrorResponse(404, "not_found", "Order not found", "error"); + } + + if (orderSummary.patient_uid !== patientUID) { + commons.logError("result-status-lambda", "Patient UID in Task does not match order record", { + orderUID, + }); + return createFhirErrorResponse( + 403, + "forbidden", + "Patient UID does not match order record", + "error", + ); + } + + const correlationId = getCorrelationIdFromEventHeaders(event); + + try { + await resultService.updateResultStatus(orderUID, ResultStatus.Result_Available, correlationId); + } catch (error) { + commons.logError("result-status-lambda", "Failed to update result status in database", { + error, + orderUID, + }); + return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); + } + + return createFhirResponse(201, task); +}; diff --git a/lambdas/src/result-status-lambda/init.test.ts b/lambdas/src/result-status-lambda/init.test.ts new file mode 100644 index 000000000..af8695917 --- /dev/null +++ b/lambdas/src/result-status-lambda/init.test.ts @@ -0,0 +1,147 @@ +import { ConsoleCommons } from "../lib/commons"; +import { postgresConfigFromEnv } from "../lib/db/db-config"; +import { OrderService } from "../lib/db/order-db"; +import { ResultService } from "../lib/db/result-db"; +import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; +import { buildEnvironment, init } from "./init"; + +// Mock all external dependencies +jest.mock("../lib/db/db-client"); +jest.mock("../lib/db/db-config"); +jest.mock("../lib/secrets/secrets-manager-client"); +jest.mock("../lib/commons"); +jest.mock("../lib/db/result-db"); +jest.mock("../lib/db/order-db"); + +describe("init", () => { + const originalEnv = process.env; + + const mockEnvVariables = { + DB_USERNAME: "test-username", + DB_ADDRESS: "test-address", + DB_PORT: "5432", + DB_NAME: "test-database", + DB_SCHEMA: "test-schema", + DB_SECRET_NAME: "test-secret-name", + }; + + // This represents the return value of postgresConfigFromEnv(secretsClient) + const mockPostgresConfig = { + user: "test-user", + host: "test-host", + port: 5432, + database: "test-db", + password: jest.fn().mockResolvedValue("test-password"), + }; + + beforeEach(() => { + jest.clearAllMocks(); + // Reset process.env to a clean state + process.env = { ...originalEnv }; + // Set default mock environment variables + Object.assign(process.env, mockEnvVariables); + + (postgresConfigFromEnv as jest.Mock).mockReturnValue(mockPostgresConfig); + }); + + afterEach(() => { + // Restore original env + process.env = originalEnv; + }); + + describe("successful initialization", () => { + beforeEach(async () => { + jest.clearAllMocks(); + (postgresConfigFromEnv as jest.Mock).mockReturnValue(mockPostgresConfig); + }); + + it("should initialize all components with correct configuration", async () => { + process.env.AWS_REGION = "eu-west-2"; + + const result = init(); + + expect(result).toHaveProperty("commons"); + expect(result).toHaveProperty("resultService"); + expect(result).toHaveProperty("orderService"); + expect(result.commons).toBeInstanceOf(ConsoleCommons); + expect(result.resultService).toBeInstanceOf(ResultService); + expect(result.orderService).toBeInstanceOf(OrderService); + }); + + it("should create AwsSecretsClient with AWS_REGION when set", () => { + process.env.AWS_REGION = "us-east-1"; + + jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { init: initModule } = require("./init"); + initModule(); + expect(AwsSecretsClient).toHaveBeenCalledWith("us-east-1"); + }); + }); + + it("should create AwsSecretsClient with AWS_DEFAULT_REGION when AWS_REGION is not set", () => { + delete process.env.AWS_REGION; + process.env.AWS_DEFAULT_REGION = "us-west-2"; + + jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { init: initModule } = require("./init"); + initModule(); + expect(AwsSecretsClient).toHaveBeenCalledWith("us-west-2"); + }); + }); + + it("should default to eu-west-2 when neither AWS_REGION nor AWS_DEFAULT_REGION is set", () => { + delete process.env.AWS_REGION; + delete process.env.AWS_DEFAULT_REGION; + + jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { init: initModule } = require("./init"); + initModule(); + expect(AwsSecretsClient).toHaveBeenCalledWith("eu-west-2"); + }); + }); + + it("should return an Environment object with all required properties", () => { + const result = init(); + + expect(result).toEqual({ + commons: expect.any(ConsoleCommons), + resultService: expect.any(ResultService), + orderService: expect.any(OrderService), + }); + }); + }); + + describe("Integration of components", () => { + it("should create ResultService and OrderService with the same dbClient instance", () => { + buildEnvironment(); + + // Extract the dbClient instances from the mocked constructors + const resultServiceDbClient = (ResultService as jest.Mock).mock.calls[0][0]; + const orderServiceDbClient = (OrderService as jest.Mock).mock.calls[0][0]; + + expect(resultServiceDbClient).toBe(orderServiceDbClient); + }); + + it("should create ResultService and OrderService with the same commons instance", () => { + buildEnvironment(); + + // Extract the commons instances from the mocked constructors + const resultServiceCommons = (ResultService as jest.Mock).mock.calls[0][1]; + const orderServiceCommons = (OrderService as jest.Mock).mock.calls[0][1]; + + expect(resultServiceCommons).toBe(orderServiceCommons); + }); + }); + + describe("singleton behavior", () => { + it("should return the same Environment instance on multiple calls to init", () => { + const env1 = init(); + const env2 = init(); + + expect(env1).toBe(env2); + }); + }); +}); diff --git a/lambdas/src/result-status-lambda/init.ts b/lambdas/src/result-status-lambda/init.ts new file mode 100644 index 000000000..6c1ca5197 --- /dev/null +++ b/lambdas/src/result-status-lambda/init.ts @@ -0,0 +1,34 @@ +import { Commons, ConsoleCommons } from "../lib/commons"; +import { PostgresDbClient } from "../lib/db/db-client"; +import { postgresConfigFromEnv } from "../lib/db/db-config"; +import { OrderService } from "../lib/db/order-db"; +import { ResultService } from "../lib/db/result-db"; +import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; + +export interface Environment { + commons: Commons; + resultService: ResultService; + orderService: OrderService; +} + +export function buildEnvironment(): Environment { + const commons = new ConsoleCommons(); + const awsRegion = process.env.AWS_REGION || process.env.AWS_DEFAULT_REGION || "eu-west-2"; + const secretsClient = new AwsSecretsClient(awsRegion); + const dbClient = new PostgresDbClient(postgresConfigFromEnv(secretsClient)); + const resultService = new ResultService(dbClient, commons); + const orderService = new OrderService(dbClient, commons); + + return { + commons, + resultService, + orderService, + }; +} + +let _env: Environment | undefined; + +export function init(): Environment { + _env ??= buildEnvironment(); + return _env; +} diff --git a/lambdas/src/result-status-lambda/models.ts b/lambdas/src/result-status-lambda/models.ts new file mode 100644 index 000000000..21040b000 --- /dev/null +++ b/lambdas/src/result-status-lambda/models.ts @@ -0,0 +1,28 @@ +import { z } from "zod"; + +import { + FHIRCodeableConceptSchema, + FHIRIdentifierSchema, + FHIRReferenceSchema, + FHIRTaskSchema, +} from "../lib/models/fhir/fhir-schemas"; + +const resultStatusFHIRCodeableConceptSchema = FHIRCodeableConceptSchema.extend({ + coding: z + .array( + z.object({ + system: z.string().optional(), + code: z.literal("result-available"), + display: z.string().optional(), + }), + ) + .min(1) + .max(1), +}); + +export const resultStatusFHIRTaskSchema = FHIRTaskSchema.extend({ + identifier: z.array(FHIRIdentifierSchema).min(1).max(1), + for: FHIRReferenceSchema, + businessStatus: resultStatusFHIRCodeableConceptSchema, + basedOn: z.array(FHIRReferenceSchema), +}); From 9988b2eaf7a8595da25cf3a5ef322eb75f509c79 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Mon, 13 Apr 2026 16:08:53 +0100 Subject: [PATCH 02/16] address copilot feedback --- lambdas/src/lib/db/result-db.test.ts | 16 +++++++ lambdas/src/lib/db/result-db.ts | 1 + .../cors-configuration.ts | 12 +++++ .../src/result-status-lambda/index.test.ts | 48 ++++++++++++------- lambdas/src/result-status-lambda/index.ts | 40 +++++++++++++--- .../{models.ts => schemas.ts} | 0 6 files changed, 92 insertions(+), 25 deletions(-) create mode 100644 lambdas/src/result-status-lambda/cors-configuration.ts rename lambdas/src/result-status-lambda/{models.ts => schemas.ts} (100%) diff --git a/lambdas/src/lib/db/result-db.test.ts b/lambdas/src/lib/db/result-db.test.ts index 6b39f151c..a2515d70a 100644 --- a/lambdas/src/lib/db/result-db.test.ts +++ b/lambdas/src/lib/db/result-db.test.ts @@ -51,5 +51,21 @@ describe("ResultService", () => { resultService.updateResultStatus(orderUid, ResultStatus.Result_Available, correlationId), ).rejects.toThrow("Database error"); }); + + it("should handle idempotent updates correctly", async () => { + // Simulate a conflict due to duplicate correlation ID + mockQuery.mockResolvedValue({ rowCount: 0 }); // No rows inserted due to conflict + + await resultService.updateResultStatus( + orderUid, + ResultStatus.Result_Available, + correlationId, + ); + + expect(mockDbClient.query).toHaveBeenCalledWith( + expect.stringContaining("INSERT INTO result_status"), + [orderUid, ResultStatus.Result_Available, correlationId], + ); + }); }); }); diff --git a/lambdas/src/lib/db/result-db.ts b/lambdas/src/lib/db/result-db.ts index 0664363e8..c6af3e26e 100644 --- a/lambdas/src/lib/db/result-db.ts +++ b/lambdas/src/lib/db/result-db.ts @@ -18,6 +18,7 @@ export class ResultService { const query = ` INSERT INTO result_status (order_uid, status, correlation_id) VALUES ($1::uuid, $2, $3) + ON CONFLICT (correlation_id) DO NOTHING `; try { await this.dbClient.query(query, [orderUid, status, correlationId]); diff --git a/lambdas/src/result-status-lambda/cors-configuration.ts b/lambdas/src/result-status-lambda/cors-configuration.ts new file mode 100644 index 000000000..bbdcb07d0 --- /dev/null +++ b/lambdas/src/result-status-lambda/cors-configuration.ts @@ -0,0 +1,12 @@ +import { type Options } from "@middy/http-cors"; + +import { defaultCorsOptions as sharedDefaultCorsOptions } from "../lib/security/cors-configuration"; + +const customCorsOptions: Options = { + methods: "POST, OPTIONS", +}; + +export const corsOptions: Options = { + ...sharedDefaultCorsOptions, + ...customCorsOptions, +}; diff --git a/lambdas/src/result-status-lambda/index.test.ts b/lambdas/src/result-status-lambda/index.test.ts index 267626dbd..74331b89d 100644 --- a/lambdas/src/result-status-lambda/index.test.ts +++ b/lambdas/src/result-status-lambda/index.test.ts @@ -2,7 +2,7 @@ import { APIGatewayProxyEvent } from "aws-lambda"; import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; import { ResultStatus } from "../lib/types/status"; -import { handler } from "./index"; +import { lambdaHandler } from "./index"; const mockLogInfo = jest.fn(); const mockLogDebug = jest.fn(); @@ -81,7 +81,7 @@ describe("result-status-lambda handler", () => { describe("request body parsing", () => { it("returns 400 when body is null", async () => { - const res = await handler(makeEvent(null)); + const res = await lambdaHandler(makeEvent(null)); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -93,7 +93,7 @@ describe("result-status-lambda handler", () => { }); it("returns 400 when body is invalid JSON", async () => { - const res = await handler(makeEvent("not-valid-json")); + const res = await lambdaHandler(makeEvent("not-valid-json")); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -104,8 +104,20 @@ describe("result-status-lambda handler", () => { ); }); + it("returns 400 when correlation ID header is missing", async () => { + const res = await lambdaHandler(makeEvent(JSON.stringify(validTask), {})); + + expect(res.statusCode).toBe(400); + expect(createFhirErrorResponse).toHaveBeenCalledWith( + 400, + "invalid", + "Invalid correlation ID in headers", + "error", + ); + }); + it("returns 400 when task fails schema validation", async () => { - const res = await handler(makeEvent(JSON.stringify({ resourceType: "Task" }))); + const res = await lambdaHandler(makeEvent(JSON.stringify({ resourceType: "Task" }))); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -119,7 +131,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when resourceType is not Task", async () => { const invalidTask = { ...validTask, resourceType: "Observation" }; - const res = await handler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); expect(res.statusCode).toBe(400); }); @@ -130,7 +142,7 @@ describe("result-status-lambda handler", () => { businessStatus: { coding: [{ code: ResultStatus.Result_Withheld }] }, }; - const res = await handler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); expect(res.statusCode).toBe(400); }); @@ -138,7 +150,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when identifier array is empty", async () => { const invalidTask = { ...validTask, identifier: [] }; - const res = await handler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); expect(res.statusCode).toBe(400); }); @@ -148,7 +160,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when for.reference has no slash (single segment)", async () => { const invalidTask = { ...validTask, for: { reference: VALID_PATIENT_UUID } }; - const res = await handler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -165,7 +177,7 @@ describe("result-status-lambda handler", () => { for: { reference: `Patient/${VALID_PATIENT_UUID}/extra` }, }; - const res = await handler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -179,7 +191,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when patient ID in for.reference is not a valid UUID", async () => { const invalidTask = { ...validTask, for: { reference: "Patient/not-a-uuid" } }; - const res = await handler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -193,7 +205,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when order identifier value is not a valid UUID", async () => { const invalidTask = { ...validTask, identifier: [{ value: "not-a-uuid" }] }; - const res = await handler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -209,7 +221,7 @@ describe("result-status-lambda handler", () => { it("returns 500 when orderService.retrievePatientIdFromOrder throws", async () => { mockRetrievePatientIdFromOrder.mockRejectedValueOnce(new Error("DB connection failed")); - const res = await handler(makeEvent(JSON.stringify(validTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(validTask))); expect(res.statusCode).toBe(500); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -223,12 +235,12 @@ describe("result-status-lambda handler", () => { it("returns 404 when order is not found", async () => { mockRetrievePatientIdFromOrder.mockResolvedValueOnce(null); - const res = await handler(makeEvent(JSON.stringify(validTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(validTask))); expect(res.statusCode).toBe(404); expect(createFhirErrorResponse).toHaveBeenCalledWith( 404, - "not_found", + "not-found", "Order not found", "error", ); @@ -242,7 +254,7 @@ describe("result-status-lambda handler", () => { patient_uid: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", }); - const res = await handler(makeEvent(JSON.stringify(validTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(validTask))); expect(res.statusCode).toBe(403); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -258,7 +270,7 @@ describe("result-status-lambda handler", () => { it("returns 500 when resultService.updateResultStatus throws", async () => { mockUpdateResultStatus.mockRejectedValueOnce(new Error("DB write failed")); - const res = await handler(makeEvent(JSON.stringify(validTask), validEventHeaders)); + const res = await lambdaHandler(makeEvent(JSON.stringify(validTask), validEventHeaders)); expect(res.statusCode).toBe(500); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -270,7 +282,7 @@ describe("result-status-lambda handler", () => { }); it("calls updateResultStatus with correct arguments", async () => { - await handler(makeEvent(JSON.stringify(validTask), validEventHeaders)); + await lambdaHandler(makeEvent(JSON.stringify(validTask), validEventHeaders)); expect(mockUpdateResultStatus).toHaveBeenCalledWith( VALID_ORDER_UUID, @@ -282,7 +294,7 @@ describe("result-status-lambda handler", () => { describe("success", () => { it("returns 201 with the original task on success", async () => { - const res = await handler(makeEvent(JSON.stringify(validTask), validEventHeaders)); + const res = await lambdaHandler(makeEvent(JSON.stringify(validTask), validEventHeaders)); expect(res.statusCode).toBe(201); expect(createFhirResponse).toHaveBeenCalledWith( diff --git a/lambdas/src/result-status-lambda/index.ts b/lambdas/src/result-status-lambda/index.ts index f3a62adfa..712388f05 100644 --- a/lambdas/src/result-status-lambda/index.ts +++ b/lambdas/src/result-status-lambda/index.ts @@ -1,17 +1,23 @@ +import middy from "@middy/core"; +import cors from "@middy/http-cors"; +import httpErrorHandler from "@middy/http-error-handler"; +import httpSecurityHeaders from "@middy/http-security-headers"; import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; -import { ConsoleCommons } from "../lib/commons"; +import { Commons } from "../lib/commons"; import { type OrderPatientReference } from "../lib/db/order-db"; import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; +import { securityHeaders } from "../lib/http/security-headers"; import { type FHIRTask } from "../lib/models/fhir/fhir-service-request-type"; import { ResultStatus } from "../lib/types/status"; import { getCorrelationIdFromEventHeaders, isUUID } from "../lib/utils/utils"; import { generateReadableError } from "../lib/utils/validation-utils"; +import { corsOptions } from "./cors-configuration"; import { init } from "./init"; -import { resultStatusFHIRTaskSchema } from "./models"; +import { resultStatusFHIRTaskSchema } from "./schemas"; -function parseAndValidateTask(body: string | null, commons: ConsoleCommons): FHIRTask { - let parsedTask: string; +function parseAndValidateTask(body: string | null, commons: Commons): FHIRTask { + let parsedTask: unknown; if (!body) { commons.logError("result-status-lambda", "Missing request body"); @@ -69,7 +75,9 @@ function extractOrderUidFromFHIRTask(task: FHIRTask): string { * Accepts FHIR Task resources and updates result status on database after validation and business logic checks. * Returns appropriate FHIR responses for success and error cases. */ -export const handler = async (event: APIGatewayProxyEvent): Promise => { +export const lambdaHandler = async ( + event: APIGatewayProxyEvent, +): Promise => { const { commons, resultService, orderService } = init(); commons.logInfo("result-status-lambda", "Received result status request", { path: event.path, @@ -112,7 +120,7 @@ export const handler = async (event: APIGatewayProxyEvent): Promise Date: Mon, 13 Apr 2026 16:26:52 +0100 Subject: [PATCH 03/16] add init reject retry test coverage --- lambdas/src/result-status-lambda/init.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lambdas/src/result-status-lambda/init.test.ts b/lambdas/src/result-status-lambda/init.test.ts index af8695917..009c812fe 100644 --- a/lambdas/src/result-status-lambda/init.test.ts +++ b/lambdas/src/result-status-lambda/init.test.ts @@ -5,6 +5,8 @@ import { ResultService } from "../lib/db/result-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; import { buildEnvironment, init } from "./init"; +const mockGetSecretValue = jest.fn(); + // Mock all external dependencies jest.mock("../lib/db/db-client"); jest.mock("../lib/db/db-config"); @@ -13,6 +15,12 @@ jest.mock("../lib/commons"); jest.mock("../lib/db/result-db"); jest.mock("../lib/db/order-db"); +jest.mock("../lib/secrets/secrets-manager-client", () => ({ + AwsSecretsClient: jest.fn().mockImplementation(() => ({ + getSecretValue: mockGetSecretValue, + })), +})); + describe("init", () => { const originalEnv = process.env; @@ -144,4 +152,23 @@ describe("init", () => { expect(env1).toBe(env2); }); }); + + describe("rejection retry", () => { + it("should allow retry after buildEnvironment throws", () => { + jest.isolateModules(() => { + jest.clearAllMocks(); + (OrderService as jest.Mock).mockImplementationOnce(() => { + throw new Error("DB connection failed"); + }); + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { init: singletonInit } = require("./init"); + + expect(() => singletonInit()).toThrow("DB connection failed"); + + // _env was never assigned (??= only assigns if the expression completes) + const result = singletonInit(); + expect(result).toBeTruthy(); + }); + }); + }); }); From 867697807430b89c47f0448add666e49f05ecf69 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Mon, 13 Apr 2026 17:13:24 +0100 Subject: [PATCH 04/16] copilot feedback --- lambdas/src/lib/db/order-db.test.ts | 21 ++----- lambdas/src/lib/db/order-db.ts | 11 ++-- lambdas/src/lib/db/result-db.test.ts | 9 +-- lambdas/src/lib/db/result-db.ts | 9 +-- lambdas/src/order-result-lambda/init.ts | 2 +- .../src/result-status-lambda/index.test.ts | 46 +++++++------- lambdas/src/result-status-lambda/index.ts | 61 ++++++++++--------- lambdas/src/result-status-lambda/init.test.ts | 23 ------- lambdas/src/result-status-lambda/init.ts | 8 +-- 9 files changed, 72 insertions(+), 118 deletions(-) diff --git a/lambdas/src/lib/db/order-db.test.ts b/lambdas/src/lib/db/order-db.test.ts index 04cf47ed5..1c54d4416 100644 --- a/lambdas/src/lib/db/order-db.test.ts +++ b/lambdas/src/lib/db/order-db.test.ts @@ -1,4 +1,3 @@ -import { Commons } from "../commons"; import { OrderStatus, ResultStatus } from "../types/status"; import { OrderResultSummary, OrderService } from "./order-db"; @@ -6,7 +5,6 @@ const normalizeWhitespace = (sql: string): string => sql.replace(/\s+/g, " ").tr describe("OrderService", () => { let dbClient: any; - let commons: Pick; let orderService: OrderService; beforeEach(() => { @@ -14,10 +12,7 @@ describe("OrderService", () => { query: jest.fn(), withTransaction: jest.fn(), }; - commons = { - logError: jest.fn(), - }; - orderService = new OrderService(dbClient, commons as any as Commons); + orderService = new OrderService(dbClient); }); describe("retrieveOrderDetails", () => { @@ -82,14 +77,10 @@ describe("OrderService", () => { normalizeWhitespace(expectedRetrieveOrderDetailsQuery), ); expect(dbClient.query.mock.calls[0][1]).toEqual(["order-500"]); - expect(commons.logError).toHaveBeenCalledWith( - "order-db", - "Failed to retrieve order details", - { - error, - orderUid: "order-500", - }, - ); + expect(console.error).toHaveBeenCalledWith("order-db", "Failed to retrieve order details", { + error, + orderUid: "order-500", + }); }); }); @@ -208,7 +199,7 @@ describe("OrderService", () => { ), ).rejects.toThrow(error); - expect(commons.logError).toHaveBeenCalledWith( + expect(console.error).toHaveBeenCalledWith( "order-db", "Failed to update order and result status", { diff --git a/lambdas/src/lib/db/order-db.ts b/lambdas/src/lib/db/order-db.ts index 67f784754..8f9fae984 100644 --- a/lambdas/src/lib/db/order-db.ts +++ b/lambdas/src/lib/db/order-db.ts @@ -1,4 +1,3 @@ -import { Commons } from "../commons"; import { OrderStatus, ResultStatus } from "../types/status"; import { DBClient } from "./db-client"; @@ -18,10 +17,8 @@ export interface OrderPatientReference { export class OrderService { private readonly dbClient: DBClient; - private readonly commons: Commons; - constructor(dbClient: DBClient, commons: Commons) { + constructor(dbClient: DBClient) { this.dbClient = dbClient; - this.commons = commons; } async retrieveOrderDetails(orderUid: string): Promise { @@ -45,7 +42,7 @@ export class OrderService { const result = await this.dbClient.query(query, [orderUid]); return result.rows[0] || null; } catch (error) { - this.commons.logError("order-db", "Failed to retrieve order details", { error, orderUid }); + console.error("order-db", "Failed to retrieve order details", { error, orderUid }); throw error; } } @@ -64,7 +61,7 @@ export class OrderService { const result = await this.dbClient.query(query, [orderUid]); return result.rows[0] || null; } catch (error) { - this.commons.logError("order-db", "Failed to retrieve order-patient association", { + console.error("order-db", "Failed to retrieve order-patient association", { error, orderUid, }); @@ -99,7 +96,7 @@ export class OrderService { await dbClient.query(resultStatusQuery, [orderUid, resultStatus, correlationId]); }); } catch (error) { - this.commons.logError("order-db", "Failed to update order and result status", { + console.error("order-db", "Failed to update order and result status", { error, orderUid, }); diff --git a/lambdas/src/lib/db/result-db.test.ts b/lambdas/src/lib/db/result-db.test.ts index a2515d70a..d9a5d66ec 100644 --- a/lambdas/src/lib/db/result-db.test.ts +++ b/lambdas/src/lib/db/result-db.test.ts @@ -1,4 +1,3 @@ -import { Commons } from "../commons"; import { ResultStatus } from "../types/status"; import { DBClient } from "./db-client"; import { ResultService } from "./result-db"; @@ -11,12 +10,6 @@ const mockDbClient: DBClient = { withTransaction: jest.fn(), }; -const mockCommons: Commons = { - logError: jest.fn(), - logInfo: jest.fn(), - logDebug: jest.fn(), -}; - const orderUid = "order-123"; const correlationId = "corr-xyz"; @@ -26,7 +19,7 @@ describe("ResultService", () => { beforeEach(() => { jest.clearAllMocks(); mockQuery.mockReset(); - resultService = new ResultService(mockDbClient, mockCommons); + resultService = new ResultService(mockDbClient); }); describe("updateResultStatus", () => { diff --git a/lambdas/src/lib/db/result-db.ts b/lambdas/src/lib/db/result-db.ts index c6af3e26e..1d0e11e29 100644 --- a/lambdas/src/lib/db/result-db.ts +++ b/lambdas/src/lib/db/result-db.ts @@ -1,19 +1,16 @@ -import { Commons } from "../commons"; import { ResultStatus } from "../types/status"; import { DBClient } from "./db-client"; export class ResultService { private readonly dbClient: DBClient; - private readonly commons: Commons; - constructor(dbClient: DBClient, commons: Commons) { + constructor(dbClient: DBClient) { this.dbClient = dbClient; - this.commons = commons; } async updateResultStatus( orderUid: string, status: ResultStatus, - correlationId: string | null, + correlationId: string, ): Promise { const query = ` INSERT INTO result_status (order_uid, status, correlation_id) @@ -23,7 +20,7 @@ export class ResultService { try { await this.dbClient.query(query, [orderUid, status, correlationId]); } catch (error) { - this.commons.logError("result-db", "Failed to update result status", { + console.error("Failed to update result status", { error, orderUid, status, diff --git a/lambdas/src/order-result-lambda/init.ts b/lambdas/src/order-result-lambda/init.ts index 5c8f5b929..53588c095 100644 --- a/lambdas/src/order-result-lambda/init.ts +++ b/lambdas/src/order-result-lambda/init.ts @@ -25,7 +25,7 @@ export function buildEnvironment(): Environment { const homeTestBaseUrl = retrieveMandatoryEnvVariable("HOME_TEST_BASE_URL"); const secretsClient = new AwsSecretsClient(awsRegion); const dbClient = new PostgresDbClient(postgresConfigFromEnv(secretsClient)); - const orderService = new OrderService(dbClient, commons); + const orderService = new OrderService(dbClient); const orderStatusDb = new OrderStatusService(dbClient); const patientDbClient = new PatientDbClient(dbClient); const orderDbClient = new OrderDbClient(dbClient); diff --git a/lambdas/src/result-status-lambda/index.test.ts b/lambdas/src/result-status-lambda/index.test.ts index 74331b89d..6da487519 100644 --- a/lambdas/src/result-status-lambda/index.test.ts +++ b/lambdas/src/result-status-lambda/index.test.ts @@ -4,19 +4,11 @@ import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-respons import { ResultStatus } from "../lib/types/status"; import { lambdaHandler } from "./index"; -const mockLogInfo = jest.fn(); -const mockLogDebug = jest.fn(); -const mockLogError = jest.fn(); const mockUpdateResultStatus = jest.fn(); const mockRetrievePatientIdFromOrder = jest.fn(); jest.mock("./init", () => ({ init: jest.fn(() => ({ - commons: { - logInfo: mockLogInfo, - logDebug: mockLogDebug, - logError: mockLogError, - }, resultService: { updateResultStatus: mockUpdateResultStatus, }, @@ -40,6 +32,7 @@ jest.mock("../lib/fhir-response", () => ({ const VALID_ORDER_UUID = "123a1234-a12b-1234-abcd-1234567890ab"; const VALID_PATIENT_UUID = "123a1234-a12b-1234-abcd-1234567890ab"; const VALID_CORRELATION_ID = "123a1234-a12b-1234-abcd-1234567890ab"; +const VALID_HEADERS = { "X-Correlation-ID": VALID_CORRELATION_ID }; const validTask = { resourceType: "Task", @@ -66,9 +59,6 @@ describe("result-status-lambda handler", () => { beforeEach(() => { jest.clearAllMocks(); - mockLogInfo.mockReset(); - mockLogDebug.mockReset(); - mockLogError.mockReset(); mockUpdateResultStatus.mockReset(); mockRetrievePatientIdFromOrder.mockReset(); @@ -81,7 +71,7 @@ describe("result-status-lambda handler", () => { describe("request body parsing", () => { it("returns 400 when body is null", async () => { - const res = await lambdaHandler(makeEvent(null)); + const res = await lambdaHandler(makeEvent(null, VALID_HEADERS)); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -93,7 +83,7 @@ describe("result-status-lambda handler", () => { }); it("returns 400 when body is invalid JSON", async () => { - const res = await lambdaHandler(makeEvent("not-valid-json")); + const res = await lambdaHandler(makeEvent("not-valid-json", VALID_HEADERS)); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -117,7 +107,9 @@ describe("result-status-lambda handler", () => { }); it("returns 400 when task fails schema validation", async () => { - const res = await lambdaHandler(makeEvent(JSON.stringify({ resourceType: "Task" }))); + const res = await lambdaHandler( + makeEvent(JSON.stringify({ resourceType: "Task" }), VALID_HEADERS), + ); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -131,7 +123,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when resourceType is not Task", async () => { const invalidTask = { ...validTask, resourceType: "Observation" }; - const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask), VALID_HEADERS)); expect(res.statusCode).toBe(400); }); @@ -142,7 +134,7 @@ describe("result-status-lambda handler", () => { businessStatus: { coding: [{ code: ResultStatus.Result_Withheld }] }, }; - const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask), VALID_HEADERS)); expect(res.statusCode).toBe(400); }); @@ -150,7 +142,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when identifier array is empty", async () => { const invalidTask = { ...validTask, identifier: [] }; - const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask), VALID_HEADERS)); expect(res.statusCode).toBe(400); }); @@ -160,7 +152,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when for.reference has no slash (single segment)", async () => { const invalidTask = { ...validTask, for: { reference: VALID_PATIENT_UUID } }; - const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask), VALID_HEADERS)); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -177,7 +169,7 @@ describe("result-status-lambda handler", () => { for: { reference: `Patient/${VALID_PATIENT_UUID}/extra` }, }; - const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask), VALID_HEADERS)); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -191,7 +183,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when patient ID in for.reference is not a valid UUID", async () => { const invalidTask = { ...validTask, for: { reference: "Patient/not-a-uuid" } }; - const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask), VALID_HEADERS)); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -205,7 +197,7 @@ describe("result-status-lambda handler", () => { it("returns 400 when order identifier value is not a valid UUID", async () => { const invalidTask = { ...validTask, identifier: [{ value: "not-a-uuid" }] }; - const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask))); + const res = await lambdaHandler(makeEvent(JSON.stringify(invalidTask), VALID_HEADERS)); expect(res.statusCode).toBe(400); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -221,7 +213,9 @@ describe("result-status-lambda handler", () => { it("returns 500 when orderService.retrievePatientIdFromOrder throws", async () => { mockRetrievePatientIdFromOrder.mockRejectedValueOnce(new Error("DB connection failed")); - const res = await lambdaHandler(makeEvent(JSON.stringify(validTask))); + const res = await lambdaHandler( + makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), + ); expect(res.statusCode).toBe(500); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -235,7 +229,9 @@ describe("result-status-lambda handler", () => { it("returns 404 when order is not found", async () => { mockRetrievePatientIdFromOrder.mockResolvedValueOnce(null); - const res = await lambdaHandler(makeEvent(JSON.stringify(validTask))); + const res = await lambdaHandler( + makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), + ); expect(res.statusCode).toBe(404); expect(createFhirErrorResponse).toHaveBeenCalledWith( @@ -254,7 +250,9 @@ describe("result-status-lambda handler", () => { patient_uid: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", }); - const res = await lambdaHandler(makeEvent(JSON.stringify(validTask))); + const res = await lambdaHandler( + makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), + ); expect(res.statusCode).toBe(403); expect(createFhirErrorResponse).toHaveBeenCalledWith( diff --git a/lambdas/src/result-status-lambda/index.ts b/lambdas/src/result-status-lambda/index.ts index 712388f05..55aed1c9f 100644 --- a/lambdas/src/result-status-lambda/index.ts +++ b/lambdas/src/result-status-lambda/index.ts @@ -4,7 +4,6 @@ import httpErrorHandler from "@middy/http-error-handler"; import httpSecurityHeaders from "@middy/http-security-headers"; import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; -import { Commons } from "../lib/commons"; import { type OrderPatientReference } from "../lib/db/order-db"; import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; import { securityHeaders } from "../lib/http/security-headers"; @@ -16,18 +15,20 @@ import { corsOptions } from "./cors-configuration"; import { init } from "./init"; import { resultStatusFHIRTaskSchema } from "./schemas"; -function parseAndValidateTask(body: string | null, commons: Commons): FHIRTask { +const name = "result-status-lambda"; + +function parseAndValidateTask(body: string | null): FHIRTask { let parsedTask: unknown; if (!body) { - commons.logError("result-status-lambda", "Missing request body"); + console.error(name, "Missing request body"); throw new Error("Request body is required"); } try { parsedTask = JSON.parse(body); } catch (error) { - commons.logError("result-status-lambda", "Invalid JSON in request body", { error }); + console.error(name, "Invalid JSON in request body", { error }); throw new Error("Invalid JSON in request body", { cause: error }); } @@ -35,7 +36,7 @@ function parseAndValidateTask(body: string | null, commons: Commons): FHIRTask { if (!validationResult.success) { const errorDetails = generateReadableError(validationResult.error); - commons.logError("result-status-lambda", "Task validation failed", { error: errorDetails }); + console.error(name, "Task validation failed", { error: errorDetails }); throw new Error(`Task validation failed: ${errorDetails}`); } @@ -78,16 +79,28 @@ function extractOrderUidFromFHIRTask(task: FHIRTask): string { export const lambdaHandler = async ( event: APIGatewayProxyEvent, ): Promise => { - const { commons, resultService, orderService } = init(); - commons.logInfo("result-status-lambda", "Received result status request", { + const { resultService, orderService } = init(); + let correlationId: string; + + try { + correlationId = getCorrelationIdFromEventHeaders(event); + } catch (error) { + console.error(name, "Failed to extract correlation ID from request headers", { + error, + }); + return createFhirErrorResponse(400, "invalid", "Invalid correlation ID in headers", "error"); + } + + console.info(name, "Received result status request", { path: event.path, method: event.httpMethod, + correlationId: correlationId, }); let task: FHIRTask; try { - task = parseAndValidateTask(event.body, commons); + task = parseAndValidateTask(event.body); } catch (error) { const message = error instanceof Error ? error.message : "Invalid request body"; return createFhirErrorResponse(400, "invalid", message, "error"); @@ -100,8 +113,9 @@ export const lambdaHandler = async ( orderUID = extractOrderUidFromFHIRTask(task); } catch (error) { const message = error instanceof Error ? error.message : "Invalid identifiers"; - commons.logError("result-status-lambda", "Failed to extract identifiers from FHIR Task", { + console.error(name, "Failed to extract identifiers from FHIR Task", { error, + correlationId, }); return createFhirErrorResponse(400, "invalid", message, "error"); } @@ -111,21 +125,26 @@ export const lambdaHandler = async ( try { orderSummary = await orderService.retrievePatientIdFromOrder(orderUID); } catch (error) { - commons.logError("result-status-lambda", "Failed to retrieve order details from database", { + console.error(name, "Failed to retrieve order details from database", { error, orderUID, + correlationId, }); return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); } if (!orderSummary) { - commons.logError("result-status-lambda", "Order not found for given order UID", { orderUID }); + console.error(name, "Order not found for given order UID", { + orderUID, + correlationId, + }); return createFhirErrorResponse(404, "not-found", "Order not found", "error"); } if (orderSummary.patient_uid !== patientUID) { - commons.logError("result-status-lambda", "Patient UID in Task does not match order record", { + console.error(name, "Patient UID in Task does not match order record", { orderUID, + correlationId, }); return createFhirErrorResponse( 403, @@ -135,27 +154,13 @@ export const lambdaHandler = async ( ); } - let correlationId: string; - - try { - correlationId = getCorrelationIdFromEventHeaders(event); - } catch (error) { - commons.logError( - "result-status-lambda", - "Failed to extract correlation ID from request headers", - { - error, - }, - ); - return createFhirErrorResponse(400, "invalid", "Invalid correlation ID in headers", "error"); - } - try { await resultService.updateResultStatus(orderUID, ResultStatus.Result_Available, correlationId); } catch (error) { - commons.logError("result-status-lambda", "Failed to update result status in database", { + console.error(name, "Failed to update result status in database", { error, orderUID, + correlationId, }); return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); } diff --git a/lambdas/src/result-status-lambda/init.test.ts b/lambdas/src/result-status-lambda/init.test.ts index 009c812fe..f6698bddc 100644 --- a/lambdas/src/result-status-lambda/init.test.ts +++ b/lambdas/src/result-status-lambda/init.test.ts @@ -1,26 +1,16 @@ -import { ConsoleCommons } from "../lib/commons"; import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; import { ResultService } from "../lib/db/result-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; import { buildEnvironment, init } from "./init"; -const mockGetSecretValue = jest.fn(); - // Mock all external dependencies jest.mock("../lib/db/db-client"); jest.mock("../lib/db/db-config"); jest.mock("../lib/secrets/secrets-manager-client"); -jest.mock("../lib/commons"); jest.mock("../lib/db/result-db"); jest.mock("../lib/db/order-db"); -jest.mock("../lib/secrets/secrets-manager-client", () => ({ - AwsSecretsClient: jest.fn().mockImplementation(() => ({ - getSecretValue: mockGetSecretValue, - })), -})); - describe("init", () => { const originalEnv = process.env; @@ -68,10 +58,8 @@ describe("init", () => { const result = init(); - expect(result).toHaveProperty("commons"); expect(result).toHaveProperty("resultService"); expect(result).toHaveProperty("orderService"); - expect(result.commons).toBeInstanceOf(ConsoleCommons); expect(result.resultService).toBeInstanceOf(ResultService); expect(result.orderService).toBeInstanceOf(OrderService); }); @@ -115,7 +103,6 @@ describe("init", () => { const result = init(); expect(result).toEqual({ - commons: expect.any(ConsoleCommons), resultService: expect.any(ResultService), orderService: expect.any(OrderService), }); @@ -132,16 +119,6 @@ describe("init", () => { expect(resultServiceDbClient).toBe(orderServiceDbClient); }); - - it("should create ResultService and OrderService with the same commons instance", () => { - buildEnvironment(); - - // Extract the commons instances from the mocked constructors - const resultServiceCommons = (ResultService as jest.Mock).mock.calls[0][1]; - const orderServiceCommons = (OrderService as jest.Mock).mock.calls[0][1]; - - expect(resultServiceCommons).toBe(orderServiceCommons); - }); }); describe("singleton behavior", () => { diff --git a/lambdas/src/result-status-lambda/init.ts b/lambdas/src/result-status-lambda/init.ts index 6c1ca5197..0f9d319f1 100644 --- a/lambdas/src/result-status-lambda/init.ts +++ b/lambdas/src/result-status-lambda/init.ts @@ -1,4 +1,3 @@ -import { Commons, ConsoleCommons } from "../lib/commons"; import { PostgresDbClient } from "../lib/db/db-client"; import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; @@ -6,21 +5,18 @@ import { ResultService } from "../lib/db/result-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; export interface Environment { - commons: Commons; resultService: ResultService; orderService: OrderService; } export function buildEnvironment(): Environment { - const commons = new ConsoleCommons(); const awsRegion = process.env.AWS_REGION || process.env.AWS_DEFAULT_REGION || "eu-west-2"; const secretsClient = new AwsSecretsClient(awsRegion); const dbClient = new PostgresDbClient(postgresConfigFromEnv(secretsClient)); - const resultService = new ResultService(dbClient, commons); - const orderService = new OrderService(dbClient, commons); + const resultService = new ResultService(dbClient); + const orderService = new OrderService(dbClient); return { - commons, resultService, orderService, }; From 678f3c43b759900d645413eb97be774aeb5b1187 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 09:41:43 +0100 Subject: [PATCH 05/16] local terraform changes --- local-environment/infra/main.tf | 34 +++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/local-environment/infra/main.tf b/local-environment/infra/main.tf index 6dff99cf4..22ca67ebe 100644 --- a/local-environment/infra/main.tf +++ b/local-environment/infra/main.tf @@ -534,6 +534,34 @@ module "order_status_lambda" { } } +module "result_status_lambda" { + source = "./modules/lambda" + + project_name = var.project_name + function_name = "result-status" + zip_path = "${path.module}/../../lambdas/dist/result-status-lambda.zip" + lambda_role_arn = aws_iam_role.lambda_role.arn + environment = var.environment + api_gateway_id = aws_api_gateway_rest_api.api.id + api_gateway_root_resource_id = aws_api_gateway_rest_api.api.root_resource_id + api_gateway_execution_arn = aws_api_gateway_rest_api.api.execution_arn + api_path = "result/status" + http_method = "POST" + lambda_role_policy_attachment = aws_iam_role_policy_attachment.lambda_basic + + environment_variables = { + NODE_OPTIONS = "--enable-source-maps" + ALLOW_ORIGIN = "http://localhost:3000" + DB_USERNAME = "app_user" + DB_ADDRESS = "postgres-db" + DB_PORT = "5432" + DB_NAME = "local_hometest_db" + DB_SCHEMA = "hometest" + DB_SECRET_NAME = "postgres-db-password" + DB_SSL = "false" + } +} + # API Gateway deployment resource "aws_api_gateway_deployment" "api_deployment" { rest_api_id = aws_api_gateway_rest_api.api.id @@ -547,7 +575,8 @@ resource "aws_api_gateway_deployment" "api_deployment" { module.order_service_lambda, module.session_lambda, module.order_status_lambda, - module.postcode_lookup_lambda + module.postcode_lookup_lambda, + module.result_status_lambda ] triggers = { @@ -560,7 +589,8 @@ resource "aws_api_gateway_deployment" "api_deployment" { module.order_service_lambda, module.session_lambda, module.order_status_lambda, - module.postcode_lookup_lambda + module.postcode_lookup_lambda, + module.result_status_lambda ])) } From a6ebb7dbf06c6f995a2d1c3b7be48de515d173f5 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 10:42:56 +0100 Subject: [PATCH 06/16] review feedback --- lambdas/src/lib/db/order-db.test.ts | 71 +------------------ lambdas/src/lib/db/order-db.ts | 30 +------- lambdas/src/lib/db/result-db.test.ts | 64 ----------------- lambdas/src/lib/db/result-db.ts | 32 --------- .../src/result-status-lambda/index.test.ts | 48 +++++++------ lambdas/src/result-status-lambda/index.ts | 38 ++++++---- lambdas/src/result-status-lambda/init.test.ts | 16 ++--- lambdas/src/result-status-lambda/init.ts | 8 +-- 8 files changed, 66 insertions(+), 241 deletions(-) delete mode 100644 lambdas/src/lib/db/result-db.test.ts delete mode 100644 lambdas/src/lib/db/result-db.ts diff --git a/lambdas/src/lib/db/order-db.test.ts b/lambdas/src/lib/db/order-db.test.ts index 1c54d4416..b350aa120 100644 --- a/lambdas/src/lib/db/order-db.test.ts +++ b/lambdas/src/lib/db/order-db.test.ts @@ -66,7 +66,7 @@ describe("OrderService", () => { expect(result).toBeNull(); }); - it("should log and rethrow when retrieving order details fails", async () => { + it("should rethrow when retrieving order details fails", async () => { const error = new Error("query failed"); dbClient.query.mockRejectedValue(error); @@ -77,64 +77,6 @@ describe("OrderService", () => { normalizeWhitespace(expectedRetrieveOrderDetailsQuery), ); expect(dbClient.query.mock.calls[0][1]).toEqual(["order-500"]); - expect(console.error).toHaveBeenCalledWith("order-db", "Failed to retrieve order details", { - error, - orderUid: "order-500", - }); - }); - }); - - describe("retrievePatientIdFromOrder", () => { - const expectedRetrievePatientIdQuery = ` - SELECT o.patient_uid, - o.order_uid - FROM test_order o - WHERE o.order_uid = $1::uuid - ORDER BY o.created_at DESC - LIMIT 1; - `; - - it("should return patient reference when found", async () => { - const mockReference = { - order_uid: "order-123", - patient_uid: "patient-abc", - }; - dbClient.query.mockResolvedValue({ rows: [mockReference] }); - - const result = await orderService.retrievePatientIdFromOrder("order-123"); - - expect(dbClient.query).toHaveBeenCalledTimes(1); - expect(normalizeWhitespace(dbClient.query.mock.calls[0][0])).toBe( - normalizeWhitespace(expectedRetrievePatientIdQuery), - ); - expect(dbClient.query.mock.calls[0][1]).toEqual(["order-123"]); - expect(result).toEqual(mockReference); - }); - - it("should return null when no order is found", async () => { - dbClient.query.mockResolvedValue({ rows: [] }); - - const result = await orderService.retrievePatientIdFromOrder("order-404"); - - expect(dbClient.query).toHaveBeenCalledTimes(1); - expect(normalizeWhitespace(dbClient.query.mock.calls[0][0])).toBe( - normalizeWhitespace(expectedRetrievePatientIdQuery), - ); - expect(dbClient.query.mock.calls[0][1]).toEqual(["order-404"]); - expect(result).toBeNull(); - }); - - it("should rethrow when retrieving patient reference fails", async () => { - const error = new Error("query failed"); - dbClient.query.mockRejectedValue(error); - - await expect(orderService.retrievePatientIdFromOrder("order-500")).rejects.toThrow(error); - - expect(dbClient.query).toHaveBeenCalledTimes(1); - expect(normalizeWhitespace(dbClient.query.mock.calls[0][0])).toBe( - normalizeWhitespace(expectedRetrievePatientIdQuery), - ); - expect(dbClient.query.mock.calls[0][1]).toEqual(["order-500"]); }); }); @@ -186,7 +128,7 @@ describe("OrderService", () => { ]); }); - it("should log and rethrow when the transaction fails", async () => { + it("should rethrow when the transaction fails", async () => { const error = new Error("transaction failed"); dbClient.withTransaction.mockRejectedValue(error); @@ -198,15 +140,6 @@ describe("OrderService", () => { "corr-1", ), ).rejects.toThrow(error); - - expect(console.error).toHaveBeenCalledWith( - "order-db", - "Failed to update order and result status", - { - error, - orderUid: "order-1", - }, - ); }); }); }); diff --git a/lambdas/src/lib/db/order-db.ts b/lambdas/src/lib/db/order-db.ts index 8f9fae984..df1e3170b 100644 --- a/lambdas/src/lib/db/order-db.ts +++ b/lambdas/src/lib/db/order-db.ts @@ -10,11 +10,6 @@ export interface OrderResultSummary { order_status_code: OrderStatus | null; } -export interface OrderPatientReference { - order_uid: string; - patient_uid: string; -} - export class OrderService { private readonly dbClient: DBClient; constructor(dbClient: DBClient) { @@ -47,28 +42,6 @@ export class OrderService { } } - async retrievePatientIdFromOrder(orderUid: string): Promise { - const query = ` - SELECT o.patient_uid, - o.order_uid - FROM test_order o - WHERE o.order_uid = $1::uuid - ORDER BY o.created_at DESC - LIMIT 1; - `; - - try { - const result = await this.dbClient.query(query, [orderUid]); - return result.rows[0] || null; - } catch (error) { - console.error("order-db", "Failed to retrieve order-patient association", { - error, - orderUid, - }); - throw error; - } - } - async updateOrderStatusAndResultStatus( orderUid: string, statusCode: OrderStatus, @@ -99,6 +72,9 @@ export class OrderService { console.error("order-db", "Failed to update order and result status", { error, orderUid, + correlationId, + statusCode, + resultStatus, }); throw error; } diff --git a/lambdas/src/lib/db/result-db.test.ts b/lambdas/src/lib/db/result-db.test.ts deleted file mode 100644 index d9a5d66ec..000000000 --- a/lambdas/src/lib/db/result-db.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { ResultStatus } from "../types/status"; -import { DBClient } from "./db-client"; -import { ResultService } from "./result-db"; - -const mockQuery = jest.fn(); - -const mockDbClient: DBClient = { - query: mockQuery, - close: jest.fn().mockResolvedValue(undefined), - withTransaction: jest.fn(), -}; - -const orderUid = "order-123"; -const correlationId = "corr-xyz"; - -describe("ResultService", () => { - let resultService: ResultService; - - beforeEach(() => { - jest.clearAllMocks(); - mockQuery.mockReset(); - resultService = new ResultService(mockDbClient); - }); - - describe("updateResultStatus", () => { - it("should execute the expected SQL statement with the correct parameters", async () => { - await resultService.updateResultStatus( - orderUid, - ResultStatus.Result_Available, - correlationId, - ); - - expect(mockDbClient.query).toHaveBeenCalledWith( - expect.stringContaining("INSERT INTO result_status"), - [orderUid, ResultStatus.Result_Available, correlationId], - ); - }); - - it("should rethrow if the database query fails", async () => { - const mockError = new Error("Database error"); - mockQuery.mockRejectedValue(mockError); - - await expect( - resultService.updateResultStatus(orderUid, ResultStatus.Result_Available, correlationId), - ).rejects.toThrow("Database error"); - }); - - it("should handle idempotent updates correctly", async () => { - // Simulate a conflict due to duplicate correlation ID - mockQuery.mockResolvedValue({ rowCount: 0 }); // No rows inserted due to conflict - - await resultService.updateResultStatus( - orderUid, - ResultStatus.Result_Available, - correlationId, - ); - - expect(mockDbClient.query).toHaveBeenCalledWith( - expect.stringContaining("INSERT INTO result_status"), - [orderUid, ResultStatus.Result_Available, correlationId], - ); - }); - }); -}); diff --git a/lambdas/src/lib/db/result-db.ts b/lambdas/src/lib/db/result-db.ts deleted file mode 100644 index 1d0e11e29..000000000 --- a/lambdas/src/lib/db/result-db.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { ResultStatus } from "../types/status"; -import { DBClient } from "./db-client"; - -export class ResultService { - private readonly dbClient: DBClient; - constructor(dbClient: DBClient) { - this.dbClient = dbClient; - } - - async updateResultStatus( - orderUid: string, - status: ResultStatus, - correlationId: string, - ): Promise { - const query = ` - INSERT INTO result_status (order_uid, status, correlation_id) - VALUES ($1::uuid, $2, $3) - ON CONFLICT (correlation_id) DO NOTHING - `; - try { - await this.dbClient.query(query, [orderUid, status, correlationId]); - } catch (error) { - console.error("Failed to update result status", { - error, - orderUid, - status, - correlationId, - }); - throw error; - } - } -} diff --git a/lambdas/src/result-status-lambda/index.test.ts b/lambdas/src/result-status-lambda/index.test.ts index 6da487519..a238d8b76 100644 --- a/lambdas/src/result-status-lambda/index.test.ts +++ b/lambdas/src/result-status-lambda/index.test.ts @@ -1,19 +1,19 @@ import { APIGatewayProxyEvent } from "aws-lambda"; import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; -import { ResultStatus } from "../lib/types/status"; +import { OrderStatus, ResultStatus } from "../lib/types/status"; import { lambdaHandler } from "./index"; -const mockUpdateResultStatus = jest.fn(); -const mockRetrievePatientIdFromOrder = jest.fn(); +const mockGetPatientIdFromOrder = jest.fn(); +const mockUpdateOrderStatusAndResultStatus = jest.fn(); jest.mock("./init", () => ({ init: jest.fn(() => ({ - resultService: { - updateResultStatus: mockUpdateResultStatus, + orderStatusService: { + getPatientIdFromOrder: mockGetPatientIdFromOrder, }, orderService: { - retrievePatientIdFromOrder: mockRetrievePatientIdFromOrder, + updateOrderStatusAndResultStatus: mockUpdateOrderStatusAndResultStatus, }, })), })); @@ -59,14 +59,14 @@ describe("result-status-lambda handler", () => { beforeEach(() => { jest.clearAllMocks(); - mockUpdateResultStatus.mockReset(); - mockRetrievePatientIdFromOrder.mockReset(); + mockGetPatientIdFromOrder.mockReset(); + mockUpdateOrderStatusAndResultStatus.mockReset(); - mockRetrievePatientIdFromOrder.mockResolvedValue({ + mockUpdateOrderStatusAndResultStatus.mockResolvedValue({ order_uid: VALID_ORDER_UUID, patient_uid: VALID_PATIENT_UUID, }); - mockUpdateResultStatus.mockResolvedValue(undefined); + mockGetPatientIdFromOrder.mockResolvedValue(undefined); }); describe("request body parsing", () => { @@ -210,8 +210,8 @@ describe("result-status-lambda handler", () => { }); describe("order lookup", () => { - it("returns 500 when orderService.retrievePatientIdFromOrder throws", async () => { - mockRetrievePatientIdFromOrder.mockRejectedValueOnce(new Error("DB connection failed")); + it("returns 500 when orderStatusService.getPatientIdFromOrder throws", async () => { + mockGetPatientIdFromOrder.mockRejectedValueOnce(new Error("DB connection failed")); const res = await lambdaHandler( makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), @@ -227,7 +227,7 @@ describe("result-status-lambda handler", () => { }); it("returns 404 when order is not found", async () => { - mockRetrievePatientIdFromOrder.mockResolvedValueOnce(null); + mockGetPatientIdFromOrder.mockResolvedValueOnce(null); const res = await lambdaHandler( makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), @@ -245,10 +245,7 @@ describe("result-status-lambda handler", () => { describe("patient authorisation", () => { it("returns 403 when patient UID in task does not match order record", async () => { - mockRetrievePatientIdFromOrder.mockResolvedValueOnce({ - order_uid: VALID_ORDER_UUID, - patient_uid: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", - }); + mockGetPatientIdFromOrder.mockResolvedValueOnce("a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"); const res = await lambdaHandler( makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), @@ -264,9 +261,13 @@ describe("result-status-lambda handler", () => { }); }); - describe("result status update", () => { - it("returns 500 when resultService.updateResultStatus throws", async () => { - mockUpdateResultStatus.mockRejectedValueOnce(new Error("DB write failed")); + describe("Order status update", () => { + beforeEach(() => { + mockGetPatientIdFromOrder.mockResolvedValue(VALID_PATIENT_UUID); + }); + + it("returns 500 when OrderService.updateOrderStatusAndResultStatus throws", async () => { + mockUpdateOrderStatusAndResultStatus.mockRejectedValueOnce(new Error("DB write failed")); const res = await lambdaHandler(makeEvent(JSON.stringify(validTask), validEventHeaders)); @@ -279,11 +280,12 @@ describe("result-status-lambda handler", () => { ); }); - it("calls updateResultStatus with correct arguments", async () => { + it("calls updateOrderStatusAndResultStatus with correct arguments", async () => { await lambdaHandler(makeEvent(JSON.stringify(validTask), validEventHeaders)); - expect(mockUpdateResultStatus).toHaveBeenCalledWith( + expect(mockUpdateOrderStatusAndResultStatus).toHaveBeenCalledWith( VALID_ORDER_UUID, + OrderStatus.Complete, ResultStatus.Result_Available, VALID_CORRELATION_ID, ); @@ -292,6 +294,8 @@ describe("result-status-lambda handler", () => { describe("success", () => { it("returns 201 with the original task on success", async () => { + mockGetPatientIdFromOrder.mockResolvedValue(VALID_PATIENT_UUID); + const res = await lambdaHandler(makeEvent(JSON.stringify(validTask), validEventHeaders)); expect(res.statusCode).toBe(201); diff --git a/lambdas/src/result-status-lambda/index.ts b/lambdas/src/result-status-lambda/index.ts index 55aed1c9f..6b4d31415 100644 --- a/lambdas/src/result-status-lambda/index.ts +++ b/lambdas/src/result-status-lambda/index.ts @@ -4,11 +4,10 @@ import httpErrorHandler from "@middy/http-error-handler"; import httpSecurityHeaders from "@middy/http-security-headers"; import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; -import { type OrderPatientReference } from "../lib/db/order-db"; import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; import { securityHeaders } from "../lib/http/security-headers"; import { type FHIRTask } from "../lib/models/fhir/fhir-service-request-type"; -import { ResultStatus } from "../lib/types/status"; +import { OrderStatus, ResultStatus } from "../lib/types/status"; import { getCorrelationIdFromEventHeaders, isUUID } from "../lib/utils/utils"; import { generateReadableError } from "../lib/utils/validation-utils"; import { corsOptions } from "./cors-configuration"; @@ -79,7 +78,7 @@ function extractOrderUidFromFHIRTask(task: FHIRTask): string { export const lambdaHandler = async ( event: APIGatewayProxyEvent, ): Promise => { - const { resultService, orderService } = init(); + const { orderService, orderStatusService } = init(); let correlationId: string; try { @@ -106,11 +105,11 @@ export const lambdaHandler = async ( return createFhirErrorResponse(400, "invalid", message, "error"); } - let patientUID: string, orderUID: string; + let patientUid: string, orderUid: string; try { - patientUID = extractPatientIdFromFHIRTask(task); - orderUID = extractOrderUidFromFHIRTask(task); + patientUid = extractPatientIdFromFHIRTask(task); + orderUid = extractOrderUidFromFHIRTask(task); } catch (error) { const message = error instanceof Error ? error.message : "Invalid identifiers"; console.error(name, "Failed to extract identifiers from FHIR Task", { @@ -120,30 +119,30 @@ export const lambdaHandler = async ( return createFhirErrorResponse(400, "invalid", message, "error"); } - let orderSummary: OrderPatientReference | null; + let patientFromOrder: string | null; try { - orderSummary = await orderService.retrievePatientIdFromOrder(orderUID); + patientFromOrder = await orderStatusService.getPatientIdFromOrder(orderUid); } catch (error) { console.error(name, "Failed to retrieve order details from database", { error, - orderUID, + orderUID: orderUid, correlationId, }); return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); } - if (!orderSummary) { + if (!patientFromOrder) { console.error(name, "Order not found for given order UID", { - orderUID, + orderUID: orderUid, correlationId, }); return createFhirErrorResponse(404, "not-found", "Order not found", "error"); } - if (orderSummary.patient_uid !== patientUID) { + if (patientFromOrder !== patientUid) { console.error(name, "Patient UID in Task does not match order record", { - orderUID, + orderUID: orderUid, correlationId, }); return createFhirErrorResponse( @@ -155,16 +154,25 @@ export const lambdaHandler = async ( } try { - await resultService.updateResultStatus(orderUID, ResultStatus.Result_Available, correlationId); + // await resultService.updateResultStatus(orderUid, ResultStatus.Result_Available, correlationId); + //TODO: update order status + await orderService.updateOrderStatusAndResultStatus( + orderUid, + OrderStatus.Complete, + ResultStatus.Result_Available, + correlationId, + ); } catch (error) { console.error(name, "Failed to update result status in database", { error, - orderUID, + orderUID: orderUid, correlationId, }); return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); } + //TODO: send notification in HOTE-982 + return createFhirResponse(201, task); }; diff --git a/lambdas/src/result-status-lambda/init.test.ts b/lambdas/src/result-status-lambda/init.test.ts index f6698bddc..3c542f819 100644 --- a/lambdas/src/result-status-lambda/init.test.ts +++ b/lambdas/src/result-status-lambda/init.test.ts @@ -1,6 +1,6 @@ import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; -import { ResultService } from "../lib/db/result-db"; +import { OrderStatusService } from "../lib/db/order-status-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; import { buildEnvironment, init } from "./init"; @@ -8,7 +8,7 @@ import { buildEnvironment, init } from "./init"; jest.mock("../lib/db/db-client"); jest.mock("../lib/db/db-config"); jest.mock("../lib/secrets/secrets-manager-client"); -jest.mock("../lib/db/result-db"); +jest.mock("../lib/db/order-status-db"); jest.mock("../lib/db/order-db"); describe("init", () => { @@ -58,9 +58,9 @@ describe("init", () => { const result = init(); - expect(result).toHaveProperty("resultService"); + expect(result).toHaveProperty("orderStatusService"); expect(result).toHaveProperty("orderService"); - expect(result.resultService).toBeInstanceOf(ResultService); + expect(result.orderStatusService).toBeInstanceOf(OrderStatusService); expect(result.orderService).toBeInstanceOf(OrderService); }); @@ -103,21 +103,21 @@ describe("init", () => { const result = init(); expect(result).toEqual({ - resultService: expect.any(ResultService), + orderStatusService: expect.any(OrderStatusService), orderService: expect.any(OrderService), }); }); }); describe("Integration of components", () => { - it("should create ResultService and OrderService with the same dbClient instance", () => { + it("should create OrderStatusService and OrderService with the same dbClient instance", () => { buildEnvironment(); // Extract the dbClient instances from the mocked constructors - const resultServiceDbClient = (ResultService as jest.Mock).mock.calls[0][0]; + const orderStatusServiceDbClient = (OrderStatusService as jest.Mock).mock.calls[0][0]; const orderServiceDbClient = (OrderService as jest.Mock).mock.calls[0][0]; - expect(resultServiceDbClient).toBe(orderServiceDbClient); + expect(orderStatusServiceDbClient).toBe(orderServiceDbClient); }); }); diff --git a/lambdas/src/result-status-lambda/init.ts b/lambdas/src/result-status-lambda/init.ts index 0f9d319f1..abd6f4cbd 100644 --- a/lambdas/src/result-status-lambda/init.ts +++ b/lambdas/src/result-status-lambda/init.ts @@ -1,24 +1,24 @@ import { PostgresDbClient } from "../lib/db/db-client"; import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; -import { ResultService } from "../lib/db/result-db"; +import { OrderStatusService } from "../lib/db/order-status-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; export interface Environment { - resultService: ResultService; orderService: OrderService; + orderStatusService: OrderStatusService; } export function buildEnvironment(): Environment { const awsRegion = process.env.AWS_REGION || process.env.AWS_DEFAULT_REGION || "eu-west-2"; const secretsClient = new AwsSecretsClient(awsRegion); const dbClient = new PostgresDbClient(postgresConfigFromEnv(secretsClient)); - const resultService = new ResultService(dbClient); const orderService = new OrderService(dbClient); + const orderStatusService = new OrderStatusService(dbClient); return { - resultService, orderService, + orderStatusService, }; } From fd0d011c56c12bb4c24917819b93b2b378f57ac1 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:39:40 +0100 Subject: [PATCH 07/16] review feedback --- lambdas/src/lib/db/order-db.ts | 2 +- lambdas/src/result-status-lambda/index.ts | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lambdas/src/lib/db/order-db.ts b/lambdas/src/lib/db/order-db.ts index df1e3170b..a39092436 100644 --- a/lambdas/src/lib/db/order-db.ts +++ b/lambdas/src/lib/db/order-db.ts @@ -76,7 +76,7 @@ export class OrderService { statusCode, resultStatus, }); - throw error; + throw new Error(`Failed to update order and result status`, { cause: error }); } } } diff --git a/lambdas/src/result-status-lambda/index.ts b/lambdas/src/result-status-lambda/index.ts index 6b4d31415..e3e7d4aae 100644 --- a/lambdas/src/result-status-lambda/index.ts +++ b/lambdas/src/result-status-lambda/index.ts @@ -126,7 +126,7 @@ export const lambdaHandler = async ( } catch (error) { console.error(name, "Failed to retrieve order details from database", { error, - orderUID: orderUid, + orderUid, correlationId, }); return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); @@ -134,7 +134,7 @@ export const lambdaHandler = async ( if (!patientFromOrder) { console.error(name, "Order not found for given order UID", { - orderUID: orderUid, + orderUid, correlationId, }); return createFhirErrorResponse(404, "not-found", "Order not found", "error"); @@ -142,7 +142,7 @@ export const lambdaHandler = async ( if (patientFromOrder !== patientUid) { console.error(name, "Patient UID in Task does not match order record", { - orderUID: orderUid, + orderUid, correlationId, }); return createFhirErrorResponse( @@ -154,8 +154,6 @@ export const lambdaHandler = async ( } try { - // await resultService.updateResultStatus(orderUid, ResultStatus.Result_Available, correlationId); - //TODO: update order status await orderService.updateOrderStatusAndResultStatus( orderUid, OrderStatus.Complete, @@ -165,14 +163,13 @@ export const lambdaHandler = async ( } catch (error) { console.error(name, "Failed to update result status in database", { error, - orderUID: orderUid, + orderUid, correlationId, }); return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); } //TODO: send notification in HOTE-982 - return createFhirResponse(201, task); }; From 797c223e5cccaa3e00ff26e6dcefd3a761823b29 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 12:08:19 +0100 Subject: [PATCH 08/16] review feedback --- lambdas/src/lib/db/order-db.test.ts | 39 ++++++++++++++++--- lambdas/src/lib/db/order-db.ts | 6 ++- lambdas/src/result-status-lambda/init.test.ts | 26 +++++-------- lambdas/src/result-status-lambda/init.ts | 3 +- 4 files changed, 50 insertions(+), 24 deletions(-) diff --git a/lambdas/src/lib/db/order-db.test.ts b/lambdas/src/lib/db/order-db.test.ts index b350aa120..625344f76 100644 --- a/lambdas/src/lib/db/order-db.test.ts +++ b/lambdas/src/lib/db/order-db.test.ts @@ -7,10 +7,14 @@ describe("OrderService", () => { let dbClient: any; let orderService: OrderService; + const mockQuery = jest.fn(); + const mockWithTransaction = jest.fn(); + beforeEach(() => { + mockQuery.mockClear(); dbClient = { - query: jest.fn(), - withTransaction: jest.fn(), + query: mockQuery, + withTransaction: mockWithTransaction, }; orderService = new OrderService(dbClient); }); @@ -98,11 +102,13 @@ describe("OrderService", () => { LIMIT 1) INSERT INTO order_status (order_uid, status_code, correlation_id) SELECT $1::uuid, $2::varchar(50), $3::uuid - WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)); + WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)) + ON CONFLICT (correlation_id) DO NOTHING; `; const expectedResultStatusQuery = ` INSERT INTO result_status (order_uid, status, correlation_id) - VALUES ($1::uuid, $2, $3::uuid);`; + VALUES ($1::uuid, $2, $3::uuid) + ON CONFLICT (correlation_id) DO NOTHING;`; await orderService.updateOrderStatusAndResultStatus( "order-1", @@ -139,7 +145,30 @@ describe("OrderService", () => { ResultStatus.Result_Available, "corr-1", ), - ).rejects.toThrow(error); + ).rejects.toThrow("Failed to update order and result status"); + }); + + it("should handle idempotent updates correctly", async () => { + // Simulate a conflict due to duplicate correlation ID — ON CONFLICT DO NOTHING returns rowCount: 0 + const tx = { + query: mockQuery.mockResolvedValue({ rows: [], rowCount: 0 }), + }; + mockWithTransaction.mockImplementation( + async (callback: (client: typeof tx) => Promise) => callback(tx), + ); + + await orderService.updateOrderStatusAndResultStatus( + "order-1", + OrderStatus.Complete, + ResultStatus.Result_Available, + "corr-1", + ); + + expect(tx.query).toHaveBeenCalledWith(expect.stringContaining("INSERT INTO result_status"), [ + "order-1", + ResultStatus.Result_Available, + "corr-1", + ]); }); }); }); diff --git a/lambdas/src/lib/db/order-db.ts b/lambdas/src/lib/db/order-db.ts index a39092436..0acb68b12 100644 --- a/lambdas/src/lib/db/order-db.ts +++ b/lambdas/src/lib/db/order-db.ts @@ -59,13 +59,15 @@ export class OrderService { LIMIT 1) INSERT INTO order_status (order_uid, status_code, correlation_id) SELECT $1::uuid, $2::varchar(50), $3::uuid - WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)); + WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)) + ON CONFLICT (correlation_id) DO NOTHING; `; await dbClient.query(orderStatusQuery, [orderUid, statusCode, correlationId]); const resultStatusQuery = ` INSERT INTO result_status (order_uid, status, correlation_id) - VALUES ($1::uuid, $2, $3::uuid);`; + VALUES ($1::uuid, $2, $3::uuid) + ON CONFLICT (correlation_id) DO NOTHING;`; await dbClient.query(resultStatusQuery, [orderUid, resultStatus, correlationId]); }); } catch (error) { diff --git a/lambdas/src/result-status-lambda/init.test.ts b/lambdas/src/result-status-lambda/init.test.ts index 3c542f819..11105b407 100644 --- a/lambdas/src/result-status-lambda/init.test.ts +++ b/lambdas/src/result-status-lambda/init.test.ts @@ -75,27 +75,13 @@ describe("init", () => { }); }); - it("should create AwsSecretsClient with AWS_DEFAULT_REGION when AWS_REGION is not set", () => { + it("should throw when AWS_REGION is not set", () => { delete process.env.AWS_REGION; - process.env.AWS_DEFAULT_REGION = "us-west-2"; jest.isolateModules(() => { // eslint-disable-next-line @typescript-eslint/no-require-imports const { init: initModule } = require("./init"); - initModule(); - expect(AwsSecretsClient).toHaveBeenCalledWith("us-west-2"); - }); - }); - - it("should default to eu-west-2 when neither AWS_REGION nor AWS_DEFAULT_REGION is set", () => { - delete process.env.AWS_REGION; - delete process.env.AWS_DEFAULT_REGION; - - jest.isolateModules(() => { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const { init: initModule } = require("./init"); - initModule(); - expect(AwsSecretsClient).toHaveBeenCalledWith("eu-west-2"); + expect(() => initModule()).toThrow("Missing value for an environment variable AWS_REGION"); }); }); @@ -110,6 +96,10 @@ describe("init", () => { }); describe("Integration of components", () => { + beforeEach(() => { + process.env.AWS_REGION = "us-east-1"; + }); + it("should create OrderStatusService and OrderService with the same dbClient instance", () => { buildEnvironment(); @@ -131,6 +121,10 @@ describe("init", () => { }); describe("rejection retry", () => { + beforeEach(() => { + process.env.AWS_REGION = "us-east-1"; + }); + it("should allow retry after buildEnvironment throws", () => { jest.isolateModules(() => { jest.clearAllMocks(); diff --git a/lambdas/src/result-status-lambda/init.ts b/lambdas/src/result-status-lambda/init.ts index abd6f4cbd..461272e5a 100644 --- a/lambdas/src/result-status-lambda/init.ts +++ b/lambdas/src/result-status-lambda/init.ts @@ -3,6 +3,7 @@ import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; import { OrderStatusService } from "../lib/db/order-status-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; +import { retrieveMandatoryEnvVariable } from "../lib/utils/utils"; export interface Environment { orderService: OrderService; @@ -10,7 +11,7 @@ export interface Environment { } export function buildEnvironment(): Environment { - const awsRegion = process.env.AWS_REGION || process.env.AWS_DEFAULT_REGION || "eu-west-2"; + const awsRegion = retrieveMandatoryEnvVariable("AWS_REGION"); const secretsClient = new AwsSecretsClient(awsRegion); const dbClient = new PostgresDbClient(postgresConfigFromEnv(secretsClient)); const orderService = new OrderService(dbClient); From 9e7b6b13c16c7dfca3ac4ea6f5edd6fde596f9b0 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 13:05:59 +0100 Subject: [PATCH 09/16] update architecture docs --- architecture/home-test-consumer-api.yaml | 216 ++++++++++++++++++----- 1 file changed, 167 insertions(+), 49 deletions(-) diff --git a/architecture/home-test-consumer-api.yaml b/architecture/home-test-consumer-api.yaml index 2cdd270b8..1e3445172 100644 --- a/architecture/home-test-consumer-api.yaml +++ b/architecture/home-test-consumer-api.yaml @@ -17,7 +17,7 @@ paths: content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIRServiceRequest' + $ref: "#/components/schemas/FHIRServiceRequest" example: resourceType: "ServiceRequest" id: "550e8400-e29b-41d4-a716-446655440000" @@ -65,12 +65,12 @@ paths: type: "Organization" display: "Test Supplier Ltd" responses: - '201': + "201": description: Order created successfully content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIRServiceRequest' + $ref: "#/components/schemas/FHIRServiceRequest" example: resourceType: "ServiceRequest" id: "550e8400-e29b-41d4-a716-446655440000" @@ -98,10 +98,10 @@ paths: code: "order-received" display: "Order received and validated" text: "order-received" - '400': - $ref: '#/components/responses/BadRequest' - '422': - $ref: '#/components/responses/UnprocessableEntity' + "400": + $ref: "#/components/responses/BadRequest" + "422": + $ref: "#/components/responses/UnprocessableEntity" /get-order: get: summary: Retrieve Orders @@ -115,7 +115,7 @@ paths: description: NHS number to retrieve orders for schema: type: string - pattern: '^[0-9]{10}$' + pattern: "^[0-9]{10}$" example: "9876543210" - name: date_of_birth in: query @@ -134,12 +134,12 @@ paths: format: uuid example: "550e8400-e29b-41d4-a716-446655440000" responses: - '200': + "200": description: Orders retrieved successfully content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIRBundleSearchsetServiceRequest' + $ref: "#/components/schemas/FHIRBundleSearchsetServiceRequest" example: resourceType: "Bundle" type: "searchset" @@ -180,10 +180,10 @@ paths: code: "dispatched" display: "Kit dispatched to patient" text: "dispatched" - '400': - $ref: '#/components/responses/BadRequest' - '404': - $ref: '#/components/responses/NotFound' + "400": + $ref: "#/components/responses/BadRequest" + "404": + $ref: "#/components/responses/NotFound" /results: get: @@ -214,7 +214,7 @@ paths: description: NHS number for identity verification schema: type: string - pattern: '^[0-9]{10}$' + pattern: "^[0-9]{10}$" example: "9876543210" - name: date_of_birth in: query @@ -225,12 +225,12 @@ paths: format: date example: "1990-01-01" responses: - '200': + "200": description: Results retrieved successfully content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIRObservation' + $ref: "#/components/schemas/FHIRObservation" example: resourceType: "Observation" id: "550e8400-e29b-41d4-a716-446655440001" @@ -261,12 +261,76 @@ paths: code: "N" display: "Normal" text: "Normal" - '400': - $ref: '#/components/responses/BadRequest' - '401': - $ref: '#/components/responses/Unauthorized' - '404': - $ref: '#/components/responses/NotFound' + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "404": + $ref: "#/components/responses/NotFound" + /result/status: + post: + summary: Update Result Status + description: Update the status of a test result + tags: + - Result Service + requestBody: + required: true + content: + application/fhir+json: + schema: + $ref: "#/components/schemas/FHIRTask" + example: + resourceType: "Task" + id: "550e8400-e29b-41d4-a716-446655440000" + status: "completed" + intent: "order" + basedOn: + - reference: "ServiceRequest/550e8400-e29b-41d4-a716-446655440000" + type: "ServiceRequest" + requester: + reference: "Organization/ORG001" + for: + reference: "#patient-1" + businessStatus: + coding: + - system: "https://fhir.hometest.nhs.uk/CodeSystem/result-business-status" + code: "result-available" + display: "Result available to patient" + text: "result-available" + lastModified: "2026-02-05T12:00:00Z" + + responses: + "201": + description: Order created successfully + content: + application/fhir+json: + schema: + $ref: "#/components/schemas/FHIRTask" + example: + resourceType: "Task" + id: "550e8400-e29b-41d4-a716-446655440000" + status: "completed" + intent: "order" + basedOn: + - reference: "ServiceRequest/550e8400-e29b-41d4-a716-446655440000" + type: "ServiceRequest" + requester: + reference: "Organization/ORG001" + for: + reference: "#patient-1" + businessStatus: + coding: + - system: "https://fhir.hometest.nhs.uk/CodeSystem/result-business-status" + code: "result-available" + display: "Result available to patient" + text: "result-available" + lastModified: "2026-02-05T12:00:00Z" + "400": + $ref: "#/components/responses/BadRequest" + "403": + $ref: "#/components/responses/Forbidden" + "404": + $ref: "#/components/responses/NotFound" components: schemas: @@ -294,7 +358,7 @@ components: type: array description: List of identifiers for this order (including external order id) items: - $ref: '#/components/schemas/FHIRIdentifier' + $ref: "#/components/schemas/FHIRIdentifier" authoredOn: type: string format: date @@ -325,23 +389,23 @@ components: type: array description: NHS number and other identifiers items: - $ref: '#/components/schemas/FHIRIdentifier' + $ref: "#/components/schemas/FHIRIdentifier" name: type: array description: Patient name(s) items: - $ref: '#/components/schemas/FHIRHumanName' + $ref: "#/components/schemas/FHIRHumanName" telecom: type: array description: Contact details - must include both phone and email for delivery coordination and customer communication minItems: 2 items: - $ref: '#/components/schemas/FHIRContactPoint' + $ref: "#/components/schemas/FHIRContactPoint" address: type: array description: Patient address(es) items: - $ref: '#/components/schemas/FHIRAddress' + $ref: "#/components/schemas/FHIRAddress" birthDate: type: string format: date @@ -355,11 +419,22 @@ components: intent: type: string description: Intent of the service request - enum: [proposal, plan, directive, order, original-order, reflex-order, filler-order, instance-order, option] + enum: + [ + proposal, + plan, + directive, + order, + original-order, + reflex-order, + filler-order, + instance-order, + option, + ] example: "order" code: allOf: - - $ref: '#/components/schemas/FHIRCodeableConcept' + - $ref: "#/components/schemas/FHIRCodeableConcept" - description: What is being requested/ordered (SNOMED CT) example: coding: @@ -369,13 +444,13 @@ components: text: "HIV antigen test" subject: allOf: - - $ref: '#/components/schemas/FHIRReference' + - $ref: "#/components/schemas/FHIRReference" - description: Individual or Entity the service is ordered for example: reference: "#patient-1" requester: allOf: - - $ref: '#/components/schemas/FHIRReference' + - $ref: "#/components/schemas/FHIRReference" - description: Individual making the request example: reference: "Organization/ORG001" @@ -384,7 +459,7 @@ components: description: Requested performer (supplier organization) items: allOf: - - $ref: '#/components/schemas/FHIRReference' + - $ref: "#/components/schemas/FHIRReference" - example: reference: "Organization/SUPP001" type: "Organization" @@ -416,7 +491,7 @@ components: example: "2026-02-05" valueCodeableConcept: allOf: - - $ref: '#/components/schemas/FHIRCodeableConcept' + - $ref: "#/components/schemas/FHIRCodeableConcept" - description: Business workflow status example: coding: @@ -459,15 +534,25 @@ components: type: array description: Reference to the ServiceRequest this observation fulfills items: - $ref: '#/components/schemas/FHIRReference' + $ref: "#/components/schemas/FHIRReference" status: type: string description: Status of the observation result - enum: [registered, preliminary, final, amended, corrected, cancelled, entered-in-error, unknown] + enum: + [ + registered, + preliminary, + final, + amended, + corrected, + cancelled, + entered-in-error, + unknown, + ] example: "final" code: allOf: - - $ref: '#/components/schemas/FHIRCodeableConcept' + - $ref: "#/components/schemas/FHIRCodeableConcept" - description: Type of observation (SNOMED CT or LOINC) example: coding: @@ -477,7 +562,7 @@ components: text: "HIV antigen test" subject: allOf: - - $ref: '#/components/schemas/FHIRReference' + - $ref: "#/components/schemas/FHIRReference" - description: Who and/or what the observation is about example: reference: "Patient/123e4567-e89b-12d3-a456-426614174000" @@ -495,10 +580,10 @@ components: type: array description: Who is responsible for the observation items: - $ref: '#/components/schemas/FHIRReference' + $ref: "#/components/schemas/FHIRReference" valueCodeableConcept: allOf: - - $ref: '#/components/schemas/FHIRCodeableConcept' + - $ref: "#/components/schemas/FHIRCodeableConcept" - description: Actual result value (for coded results) example: coding: @@ -513,7 +598,7 @@ components: type: array description: High, low, normal, etc. items: - $ref: '#/components/schemas/FHIRCodeableConcept' + $ref: "#/components/schemas/FHIRCodeableConcept" referenceRange: type: array description: Provides guide for interpretation @@ -586,7 +671,7 @@ components: format: uri example: "urn:uuid:550e8400-e29b-41d4-a716-446655440000" resource: - $ref: '#/components/schemas/FHIRServiceRequest' + $ref: "#/components/schemas/FHIRServiceRequest" FHIRReference: type: object @@ -613,7 +698,7 @@ components: type: array description: Code defined by a terminology system items: - $ref: '#/components/schemas/FHIRCoding' + $ref: "#/components/schemas/FHIRCoding" text: type: string description: Plain text representation of the concept @@ -770,11 +855,44 @@ components: code: type: string description: Error or warning code - enum: [invalid, structure, required, value, invariant, security, login, unknown, expired, forbidden, suppressed, processing, not-supported, duplicate, multiple-matches, not-found, deleted, too-long, code-invalid, extension, too-costly, business-rule, conflict, transient, lock-error, no-store, exception, timeout, incomplete, throttled, informational] + enum: + [ + invalid, + structure, + required, + value, + invariant, + security, + login, + unknown, + expired, + forbidden, + suppressed, + processing, + not-supported, + duplicate, + multiple-matches, + not-found, + deleted, + too-long, + code-invalid, + extension, + too-costly, + business-rule, + conflict, + transient, + lock-error, + no-store, + exception, + timeout, + incomplete, + throttled, + informational, + ] example: "business-rule" details: allOf: - - $ref: '#/components/schemas/FHIRCodeableConcept' + - $ref: "#/components/schemas/FHIRCodeableConcept" - description: Additional details about the error example: coding: @@ -799,7 +917,7 @@ components: content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIROperationOutcome' + $ref: "#/components/schemas/FHIROperationOutcome" example: resourceType: "OperationOutcome" issue: @@ -814,7 +932,7 @@ components: content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIROperationOutcome' + $ref: "#/components/schemas/FHIROperationOutcome" example: resourceType: "OperationOutcome" issue: @@ -829,7 +947,7 @@ components: content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIROperationOutcome' + $ref: "#/components/schemas/FHIROperationOutcome" example: resourceType: "OperationOutcome" issue: @@ -844,7 +962,7 @@ components: content: application/fhir+json: schema: - $ref: '#/components/schemas/FHIROperationOutcome' + $ref: "#/components/schemas/FHIROperationOutcome" example: resourceType: "OperationOutcome" issue: From e0426d2d9e3da8b078004a33d612953a8e31fe53 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 13:34:11 +0100 Subject: [PATCH 10/16] revert some order-db changes --- lambdas/src/lib/db/order-db.test.ts | 30 +++-------------------------- lambdas/src/lib/db/order-db.ts | 7 +++---- 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/lambdas/src/lib/db/order-db.test.ts b/lambdas/src/lib/db/order-db.test.ts index 625344f76..15ba5a1ce 100644 --- a/lambdas/src/lib/db/order-db.test.ts +++ b/lambdas/src/lib/db/order-db.test.ts @@ -102,13 +102,12 @@ describe("OrderService", () => { LIMIT 1) INSERT INTO order_status (order_uid, status_code, correlation_id) SELECT $1::uuid, $2::varchar(50), $3::uuid - WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)) - ON CONFLICT (correlation_id) DO NOTHING; + WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)); `; const expectedResultStatusQuery = ` INSERT INTO result_status (order_uid, status, correlation_id) - VALUES ($1::uuid, $2, $3::uuid) - ON CONFLICT (correlation_id) DO NOTHING;`; + VALUES ($1::uuid, $2, $3::uuid); + `; await orderService.updateOrderStatusAndResultStatus( "order-1", @@ -147,28 +146,5 @@ describe("OrderService", () => { ), ).rejects.toThrow("Failed to update order and result status"); }); - - it("should handle idempotent updates correctly", async () => { - // Simulate a conflict due to duplicate correlation ID — ON CONFLICT DO NOTHING returns rowCount: 0 - const tx = { - query: mockQuery.mockResolvedValue({ rows: [], rowCount: 0 }), - }; - mockWithTransaction.mockImplementation( - async (callback: (client: typeof tx) => Promise) => callback(tx), - ); - - await orderService.updateOrderStatusAndResultStatus( - "order-1", - OrderStatus.Complete, - ResultStatus.Result_Available, - "corr-1", - ); - - expect(tx.query).toHaveBeenCalledWith(expect.stringContaining("INSERT INTO result_status"), [ - "order-1", - ResultStatus.Result_Available, - "corr-1", - ]); - }); }); }); diff --git a/lambdas/src/lib/db/order-db.ts b/lambdas/src/lib/db/order-db.ts index 0acb68b12..01bb87031 100644 --- a/lambdas/src/lib/db/order-db.ts +++ b/lambdas/src/lib/db/order-db.ts @@ -59,15 +59,14 @@ export class OrderService { LIMIT 1) INSERT INTO order_status (order_uid, status_code, correlation_id) SELECT $1::uuid, $2::varchar(50), $3::uuid - WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)) - ON CONFLICT (correlation_id) DO NOTHING; + WHERE NOT EXISTS (SELECT 1 FROM latest WHERE latest.status_code = $2::varchar(50)); `; await dbClient.query(orderStatusQuery, [orderUid, statusCode, correlationId]); const resultStatusQuery = ` INSERT INTO result_status (order_uid, status, correlation_id) - VALUES ($1::uuid, $2, $3::uuid) - ON CONFLICT (correlation_id) DO NOTHING;`; + VALUES ($1::uuid, $2, $3::uuid); + `; await dbClient.query(resultStatusQuery, [orderUid, resultStatus, correlationId]); }); } catch (error) { From a4d2be331f34afa04b040479fcb226706b2b2e91 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 13:40:56 +0100 Subject: [PATCH 11/16] small architecture doc update --- architecture/home-test-consumer-api.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/architecture/home-test-consumer-api.yaml b/architecture/home-test-consumer-api.yaml index 1e3445172..3d231587a 100644 --- a/architecture/home-test-consumer-api.yaml +++ b/architecture/home-test-consumer-api.yaml @@ -301,7 +301,7 @@ paths: responses: "201": - description: Order created successfully + description: Result status updated successfully content: application/fhir+json: schema: From c914fcda2552185e6bd2d921a456ef3afa7fd328 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:18:01 +0100 Subject: [PATCH 12/16] change how idempotency check is carried out --- .../src/result-status-lambda/index.test.ts | 22 +++++++++---------- lambdas/src/result-status-lambda/index.ts | 19 +++++++++++----- lambdas/src/result-status-lambda/init.test.ts | 21 ------------------ lambdas/src/result-status-lambda/init.ts | 4 ---- 4 files changed, 25 insertions(+), 41 deletions(-) diff --git a/lambdas/src/result-status-lambda/index.test.ts b/lambdas/src/result-status-lambda/index.test.ts index a238d8b76..22c2d74f9 100644 --- a/lambdas/src/result-status-lambda/index.test.ts +++ b/lambdas/src/result-status-lambda/index.test.ts @@ -4,16 +4,14 @@ import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-respons import { OrderStatus, ResultStatus } from "../lib/types/status"; import { lambdaHandler } from "./index"; -const mockGetPatientIdFromOrder = jest.fn(); const mockUpdateOrderStatusAndResultStatus = jest.fn(); +const mockRetrieveOrderDetails = jest.fn(); jest.mock("./init", () => ({ init: jest.fn(() => ({ - orderStatusService: { - getPatientIdFromOrder: mockGetPatientIdFromOrder, - }, orderService: { updateOrderStatusAndResultStatus: mockUpdateOrderStatusAndResultStatus, + retrieveOrderDetails: mockRetrieveOrderDetails, }, })), })); @@ -59,14 +57,14 @@ describe("result-status-lambda handler", () => { beforeEach(() => { jest.clearAllMocks(); - mockGetPatientIdFromOrder.mockReset(); + mockRetrieveOrderDetails.mockReset(); mockUpdateOrderStatusAndResultStatus.mockReset(); mockUpdateOrderStatusAndResultStatus.mockResolvedValue({ order_uid: VALID_ORDER_UUID, patient_uid: VALID_PATIENT_UUID, }); - mockGetPatientIdFromOrder.mockResolvedValue(undefined); + mockRetrieveOrderDetails.mockResolvedValue(undefined); }); describe("request body parsing", () => { @@ -211,7 +209,7 @@ describe("result-status-lambda handler", () => { describe("order lookup", () => { it("returns 500 when orderStatusService.getPatientIdFromOrder throws", async () => { - mockGetPatientIdFromOrder.mockRejectedValueOnce(new Error("DB connection failed")); + mockRetrieveOrderDetails.mockRejectedValueOnce(new Error("DB connection failed")); const res = await lambdaHandler( makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), @@ -227,7 +225,7 @@ describe("result-status-lambda handler", () => { }); it("returns 404 when order is not found", async () => { - mockGetPatientIdFromOrder.mockResolvedValueOnce(null); + mockRetrieveOrderDetails.mockResolvedValueOnce(null); const res = await lambdaHandler( makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), @@ -245,7 +243,9 @@ describe("result-status-lambda handler", () => { describe("patient authorisation", () => { it("returns 403 when patient UID in task does not match order record", async () => { - mockGetPatientIdFromOrder.mockResolvedValueOnce("a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"); + mockRetrieveOrderDetails.mockResolvedValueOnce({ + patientId: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", + }); const res = await lambdaHandler( makeEvent(JSON.stringify(validTask), { "X-Correlation-ID": VALID_CORRELATION_ID }), @@ -263,7 +263,7 @@ describe("result-status-lambda handler", () => { describe("Order status update", () => { beforeEach(() => { - mockGetPatientIdFromOrder.mockResolvedValue(VALID_PATIENT_UUID); + mockRetrieveOrderDetails.mockResolvedValue({ patient_uid: VALID_PATIENT_UUID }); }); it("returns 500 when OrderService.updateOrderStatusAndResultStatus throws", async () => { @@ -294,7 +294,7 @@ describe("result-status-lambda handler", () => { describe("success", () => { it("returns 201 with the original task on success", async () => { - mockGetPatientIdFromOrder.mockResolvedValue(VALID_PATIENT_UUID); + mockRetrieveOrderDetails.mockResolvedValue({ patient_uid: VALID_PATIENT_UUID }); const res = await lambdaHandler(makeEvent(JSON.stringify(validTask), validEventHeaders)); diff --git a/lambdas/src/result-status-lambda/index.ts b/lambdas/src/result-status-lambda/index.ts index e3e7d4aae..1b1c80474 100644 --- a/lambdas/src/result-status-lambda/index.ts +++ b/lambdas/src/result-status-lambda/index.ts @@ -3,6 +3,7 @@ import cors from "@middy/http-cors"; import httpErrorHandler from "@middy/http-error-handler"; import httpSecurityHeaders from "@middy/http-security-headers"; import { APIGatewayProxyEvent, APIGatewayProxyResult } from "aws-lambda"; +import { OrderResultSummary } from "src/lib/db/order-db"; import { createFhirErrorResponse, createFhirResponse } from "../lib/fhir-response"; import { securityHeaders } from "../lib/http/security-headers"; @@ -78,7 +79,7 @@ function extractOrderUidFromFHIRTask(task: FHIRTask): string { export const lambdaHandler = async ( event: APIGatewayProxyEvent, ): Promise => { - const { orderService, orderStatusService } = init(); + const { orderService } = init(); let correlationId: string; try { @@ -119,10 +120,10 @@ export const lambdaHandler = async ( return createFhirErrorResponse(400, "invalid", message, "error"); } - let patientFromOrder: string | null; + let orderFromOrderUid: OrderResultSummary | null; try { - patientFromOrder = await orderStatusService.getPatientIdFromOrder(orderUid); + orderFromOrderUid = await orderService.retrieveOrderDetails(orderUid); } catch (error) { console.error(name, "Failed to retrieve order details from database", { error, @@ -132,7 +133,7 @@ export const lambdaHandler = async ( return createFhirErrorResponse(500, "exception", "An internal error occurred", "fatal"); } - if (!patientFromOrder) { + if (!orderFromOrderUid) { console.error(name, "Order not found for given order UID", { orderUid, correlationId, @@ -140,7 +141,15 @@ export const lambdaHandler = async ( return createFhirErrorResponse(404, "not-found", "Order not found", "error"); } - if (patientFromOrder !== patientUid) { + if (orderFromOrderUid.correlation_id === correlationId) { + console.info(name, "Duplicate request detected based on correlation ID", { + orderUid, + correlationId, + }); + return createFhirResponse(200, task); + } + + if (orderFromOrderUid.patient_uid !== patientUid) { console.error(name, "Patient UID in Task does not match order record", { orderUid, correlationId, diff --git a/lambdas/src/result-status-lambda/init.test.ts b/lambdas/src/result-status-lambda/init.test.ts index 11105b407..336ded716 100644 --- a/lambdas/src/result-status-lambda/init.test.ts +++ b/lambdas/src/result-status-lambda/init.test.ts @@ -1,6 +1,5 @@ import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; -import { OrderStatusService } from "../lib/db/order-status-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; import { buildEnvironment, init } from "./init"; @@ -8,7 +7,6 @@ import { buildEnvironment, init } from "./init"; jest.mock("../lib/db/db-client"); jest.mock("../lib/db/db-config"); jest.mock("../lib/secrets/secrets-manager-client"); -jest.mock("../lib/db/order-status-db"); jest.mock("../lib/db/order-db"); describe("init", () => { @@ -58,9 +56,7 @@ describe("init", () => { const result = init(); - expect(result).toHaveProperty("orderStatusService"); expect(result).toHaveProperty("orderService"); - expect(result.orderStatusService).toBeInstanceOf(OrderStatusService); expect(result.orderService).toBeInstanceOf(OrderService); }); @@ -89,28 +85,11 @@ describe("init", () => { const result = init(); expect(result).toEqual({ - orderStatusService: expect.any(OrderStatusService), orderService: expect.any(OrderService), }); }); }); - describe("Integration of components", () => { - beforeEach(() => { - process.env.AWS_REGION = "us-east-1"; - }); - - it("should create OrderStatusService and OrderService with the same dbClient instance", () => { - buildEnvironment(); - - // Extract the dbClient instances from the mocked constructors - const orderStatusServiceDbClient = (OrderStatusService as jest.Mock).mock.calls[0][0]; - const orderServiceDbClient = (OrderService as jest.Mock).mock.calls[0][0]; - - expect(orderStatusServiceDbClient).toBe(orderServiceDbClient); - }); - }); - describe("singleton behavior", () => { it("should return the same Environment instance on multiple calls to init", () => { const env1 = init(); diff --git a/lambdas/src/result-status-lambda/init.ts b/lambdas/src/result-status-lambda/init.ts index 461272e5a..da8c97354 100644 --- a/lambdas/src/result-status-lambda/init.ts +++ b/lambdas/src/result-status-lambda/init.ts @@ -1,13 +1,11 @@ import { PostgresDbClient } from "../lib/db/db-client"; import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; -import { OrderStatusService } from "../lib/db/order-status-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; import { retrieveMandatoryEnvVariable } from "../lib/utils/utils"; export interface Environment { orderService: OrderService; - orderStatusService: OrderStatusService; } export function buildEnvironment(): Environment { @@ -15,11 +13,9 @@ export function buildEnvironment(): Environment { const secretsClient = new AwsSecretsClient(awsRegion); const dbClient = new PostgresDbClient(postgresConfigFromEnv(secretsClient)); const orderService = new OrderService(dbClient); - const orderStatusService = new OrderStatusService(dbClient); return { orderService, - orderStatusService, }; } From e9e1dd4913573f4011bd5b03674105b334299a20 Mon Sep 17 00:00:00 2001 From: Thomas Seelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:26:26 +0100 Subject: [PATCH 13/16] Potential fix for pull request finding 'Unused variable, import, function or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- lambdas/src/result-status-lambda/init.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/src/result-status-lambda/init.test.ts b/lambdas/src/result-status-lambda/init.test.ts index 336ded716..bc8f6df6f 100644 --- a/lambdas/src/result-status-lambda/init.test.ts +++ b/lambdas/src/result-status-lambda/init.test.ts @@ -1,7 +1,7 @@ import { postgresConfigFromEnv } from "../lib/db/db-config"; import { OrderService } from "../lib/db/order-db"; import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; -import { buildEnvironment, init } from "./init"; +import { init } from "./init"; // Mock all external dependencies jest.mock("../lib/db/db-client"); From 89723b55e31929781237bb56d71b599f973b3db2 Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:37:17 +0100 Subject: [PATCH 14/16] code review feedback --- architecture/home-test-consumer-api.yaml | 12 ++++++++---- lambdas/src/result-status-lambda/index.test.ts | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/architecture/home-test-consumer-api.yaml b/architecture/home-test-consumer-api.yaml index 3d231587a..1d66950ce 100644 --- a/architecture/home-test-consumer-api.yaml +++ b/architecture/home-test-consumer-api.yaml @@ -281,7 +281,9 @@ paths: $ref: "#/components/schemas/FHIRTask" example: resourceType: "Task" - id: "550e8400-e29b-41d4-a716-446655440000" + identifier: + - system: "https://fhir.hometest.nhs.uk/Id/order-id" + value: "550e8400-e29b-41d4-a716-446655440000" status: "completed" intent: "order" basedOn: @@ -290,7 +292,7 @@ paths: requester: reference: "Organization/ORG001" for: - reference: "#patient-1" + reference: "Patient/123e4567-e89b-12d3-a456-426614174000" businessStatus: coding: - system: "https://fhir.hometest.nhs.uk/CodeSystem/result-business-status" @@ -308,7 +310,9 @@ paths: $ref: "#/components/schemas/FHIRTask" example: resourceType: "Task" - id: "550e8400-e29b-41d4-a716-446655440000" + identifier: + - system: "https://fhir.hometest.nhs.uk/Id/order-id" + value: "550e8400-e29b-41d4-a716-446655440000" status: "completed" intent: "order" basedOn: @@ -317,7 +321,7 @@ paths: requester: reference: "Organization/ORG001" for: - reference: "#patient-1" + reference: "Patient/123e4567-e89b-12d3-a456-426614174000" businessStatus: coding: - system: "https://fhir.hometest.nhs.uk/CodeSystem/result-business-status" diff --git a/lambdas/src/result-status-lambda/index.test.ts b/lambdas/src/result-status-lambda/index.test.ts index 22c2d74f9..bf881d0e0 100644 --- a/lambdas/src/result-status-lambda/index.test.ts +++ b/lambdas/src/result-status-lambda/index.test.ts @@ -208,7 +208,7 @@ describe("result-status-lambda handler", () => { }); describe("order lookup", () => { - it("returns 500 when orderStatusService.getPatientIdFromOrder throws", async () => { + it("returns 500 when orderService.retrieveOrderDetails throws", async () => { mockRetrieveOrderDetails.mockRejectedValueOnce(new Error("DB connection failed")); const res = await lambdaHandler( From 36229bfe9988b040d2c1a8fbb1c90c72fb5b19d1 Mon Sep 17 00:00:00 2001 From: Masha-kainos <256341953+Masha-kainos@users.noreply.github.com> Date: Tue, 14 Apr 2026 15:55:59 +0100 Subject: [PATCH 15/16] Fix correlationId reuse in API e2e and integration tests --- tests/tests/api/FullOrderEndpointTest.spec.ts | 12 +++++------- tests/tests/integration/UpdateResultStatus.spec.ts | 6 ++---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/tests/api/FullOrderEndpointTest.spec.ts b/tests/tests/api/FullOrderEndpointTest.spec.ts index 910d5e347..eb38cd86d 100644 --- a/tests/tests/api/FullOrderEndpointTest.spec.ts +++ b/tests/tests/api/FullOrderEndpointTest.spec.ts @@ -24,7 +24,6 @@ import { let orderId: string; let patientId: string; -let correlationId: string; const supplierId = OrderTestData.PREVENTX_SUPPLIER_ID; const POLL_TIMEOUT_MS = 15000; @@ -100,7 +99,6 @@ test.describe("Full Order E2E API", { tag: ["@API", "@db"] }, () => { const orderBody = CreateOrderResponseModel.fromJson(await orderResponse.json()); orderId = orderBody.orderUid; - correlationId = randomUUID(); patientId = (await testOrderDb.getPatientUidByNhsNumber(testedUser.nhsNumber!))!; @@ -170,7 +168,7 @@ test.describe("Full Order E2E API", { tag: ["@API", "@db"] }, () => { const testData = ResultsObservationData.buildNormalObservation(orderId, patientId, supplierId); const response = await hivResultsApi.submitTestResults( testData, - headersTestResults(correlationId), + headersTestResults(randomUUID()), ); expect(response.status()).toBe(201); @@ -181,13 +179,13 @@ test.describe("Full Order E2E API", { tag: ["@API", "@db"] }, () => { testedUser.nhsNumber!, testedUser.dob!, orderId, - correlationId, + randomUUID(), 200, ); const resultResponse = await hivResultsApi.getResult( createGetResultParams(testedUser.nhsNumber!, testedUser.dob!, orderId), - createGetResultHeaders(correlationId), + createGetResultHeaders(randomUUID()), ); hivResultsApi.validateStatus(resultResponse, 200); const resultBody = (await resultResponse.json()) as { @@ -263,7 +261,7 @@ test.describe("Full Order E2E API", { tag: ["@API", "@db"] }, () => { ); const response = await hivResultsApi.submitTestResults( testData, - headersTestResults(correlationId), + headersTestResults(randomUUID()), ); expect(response.status()).toBe(201); @@ -276,7 +274,7 @@ test.describe("Full Order E2E API", { tag: ["@API", "@db"] }, () => { testedUser.nhsNumber!, testedUser.dob!, orderId, - correlationId, + randomUUID(), 404, ); }); diff --git a/tests/tests/integration/UpdateResultStatus.spec.ts b/tests/tests/integration/UpdateResultStatus.spec.ts index 364728535..dd4b38b5b 100644 --- a/tests/tests/integration/UpdateResultStatus.spec.ts +++ b/tests/tests/integration/UpdateResultStatus.spec.ts @@ -9,7 +9,6 @@ import { headersTestResults } from "../../utils"; let orderId: string; let patientId: string; -let correlationId: string; const supplierName = "Preventx"; let supplierId: string; @@ -25,7 +24,6 @@ test.describe("Results Flow - Update Order Results Logic", { tag: "@db" }, () => orderId = result.order_uid; patientId = result.patient_uid; - correlationId = randomUUID(); console.log(`Created test order with ID: ${orderId}`); supplierId = await testOrderDb.getSupplierIdByName(supplierName); @@ -39,7 +37,7 @@ test.describe("Results Flow - Update Order Results Logic", { tag: "@db" }, () => const testData = ResultsObservationData.buildNormalObservation(orderId, patientId, supplierId); const response = await hivResultsApi.submitTestResults( testData, - headersTestResults(correlationId), + headersTestResults(randomUUID()), ); expect(response.status()).toBe(201); @@ -62,7 +60,7 @@ test.describe("Results Flow - Update Order Results Logic", { tag: "@db" }, () => const response = await hivResultsApi.submitTestResults( testData, - headersTestResults(correlationId), + headersTestResults(randomUUID()), ); expect(response.status()).toBe(201); From c898b46a0a05b118437f2a5fb1a6d2e5baf5feff Mon Sep 17 00:00:00 2001 From: Tseelig <20642658+TSeelig@users.noreply.github.com> Date: Wed, 15 Apr 2026 09:50:15 +0100 Subject: [PATCH 16/16] copilot feedback --- lambdas/src/lib/db/order-db.test.ts | 38 ++++++++++++------- lambdas/src/lib/db/order-db.ts | 20 +++++++--- .../src/result-status-lambda/index.test.ts | 4 +- lambdas/src/result-status-lambda/index.ts | 24 ++++++------ 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/lambdas/src/lib/db/order-db.test.ts b/lambdas/src/lib/db/order-db.test.ts index 15ba5a1ce..e6e439acb 100644 --- a/lambdas/src/lib/db/order-db.test.ts +++ b/lambdas/src/lib/db/order-db.test.ts @@ -21,20 +21,30 @@ describe("OrderService", () => { describe("retrieveOrderDetails", () => { const expectedRetrieveOrderDetailsQuery = ` - SELECT - o.order_uid, - o.supplier_id, - o.patient_uid, - r.status AS result_status, - r.correlation_id, - os.status_code AS order_status_code - FROM test_order o - LEFT JOIN result_status r ON o.order_uid = r.order_uid - LEFT JOIN order_status os ON o.order_uid = os.order_uid - WHERE o.order_uid = $1::uuid - ORDER BY os.created_at DESC - LIMIT 1; - `; + SELECT + o.order_uid, + o.supplier_id, + o.patient_uid, + r.status AS result_status, + r.correlation_id, + os.status_code AS order_status_code + FROM test_order o + LEFT JOIN Lateral ( + SELECT + r.status, + r.correlation_id + FROM result_status r + WHERE o.order_uid = r.order_uid + ORDER BY r.created_at DESC LIMIT 1 + ) r ON true + LEFT JOIN Lateral ( + SELECT os.status_code + FROM order_status os + WHERE o.order_uid = os.order_uid + ORDER BY os.created_at DESC LIMIT 1 + ) os ON true + WHERE o.order_uid = $1::uuid; + `; it("should return order details when found", async () => { const mockSummary: OrderResultSummary = { diff --git a/lambdas/src/lib/db/order-db.ts b/lambdas/src/lib/db/order-db.ts index 01bb87031..e5e57b39d 100644 --- a/lambdas/src/lib/db/order-db.ts +++ b/lambdas/src/lib/db/order-db.ts @@ -26,11 +26,21 @@ export class OrderService { r.correlation_id, os.status_code AS order_status_code FROM test_order o - LEFT JOIN result_status r ON o.order_uid = r.order_uid - LEFT JOIN order_status os ON o.order_uid = os.order_uid - WHERE o.order_uid = $1::uuid - ORDER BY os.created_at DESC - LIMIT 1; + LEFT JOIN Lateral ( + SELECT + r.status, + r.correlation_id + FROM result_status r + WHERE o.order_uid = r.order_uid + ORDER BY r.created_at DESC LIMIT 1 + ) r ON true + LEFT JOIN Lateral ( + SELECT os.status_code + FROM order_status os + WHERE o.order_uid = os.order_uid + ORDER BY os.created_at DESC LIMIT 1 + ) os ON true + WHERE o.order_uid = $1::uuid; `; try { diff --git a/lambdas/src/result-status-lambda/index.test.ts b/lambdas/src/result-status-lambda/index.test.ts index bf881d0e0..56fb781ff 100644 --- a/lambdas/src/result-status-lambda/index.test.ts +++ b/lambdas/src/result-status-lambda/index.test.ts @@ -64,7 +64,7 @@ describe("result-status-lambda handler", () => { order_uid: VALID_ORDER_UUID, patient_uid: VALID_PATIENT_UUID, }); - mockRetrieveOrderDetails.mockResolvedValue(undefined); + mockRetrieveOrderDetails.mockResolvedValue(null); }); describe("request body parsing", () => { @@ -244,7 +244,7 @@ describe("result-status-lambda handler", () => { describe("patient authorisation", () => { it("returns 403 when patient UID in task does not match order record", async () => { mockRetrieveOrderDetails.mockResolvedValueOnce({ - patientId: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", + patient_uid: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", }); const res = await lambdaHandler( diff --git a/lambdas/src/result-status-lambda/index.ts b/lambdas/src/result-status-lambda/index.ts index 1b1c80474..e76f4fedf 100644 --- a/lambdas/src/result-status-lambda/index.ts +++ b/lambdas/src/result-status-lambda/index.ts @@ -17,7 +17,7 @@ import { resultStatusFHIRTaskSchema } from "./schemas"; const name = "result-status-lambda"; -function parseAndValidateTask(body: string | null): FHIRTask { +function parseAndValidateTask(body: string | null, correlationId: string): FHIRTask { let parsedTask: unknown; if (!body) { @@ -28,7 +28,7 @@ function parseAndValidateTask(body: string | null): FHIRTask { try { parsedTask = JSON.parse(body); } catch (error) { - console.error(name, "Invalid JSON in request body", { error }); + console.error(name, "Invalid JSON in request body", { error, correlationId }); throw new Error("Invalid JSON in request body", { cause: error }); } @@ -36,7 +36,7 @@ function parseAndValidateTask(body: string | null): FHIRTask { if (!validationResult.success) { const errorDetails = generateReadableError(validationResult.error); - console.error(name, "Task validation failed", { error: errorDetails }); + console.error(name, "Task validation failed", { error: errorDetails, correlationId }); throw new Error(`Task validation failed: ${errorDetails}`); } @@ -100,7 +100,7 @@ export const lambdaHandler = async ( let task: FHIRTask; try { - task = parseAndValidateTask(event.body); + task = parseAndValidateTask(event.body, correlationId); } catch (error) { const message = error instanceof Error ? error.message : "Invalid request body"; return createFhirErrorResponse(400, "invalid", message, "error"); @@ -141,14 +141,6 @@ export const lambdaHandler = async ( return createFhirErrorResponse(404, "not-found", "Order not found", "error"); } - if (orderFromOrderUid.correlation_id === correlationId) { - console.info(name, "Duplicate request detected based on correlation ID", { - orderUid, - correlationId, - }); - return createFhirResponse(200, task); - } - if (orderFromOrderUid.patient_uid !== patientUid) { console.error(name, "Patient UID in Task does not match order record", { orderUid, @@ -162,6 +154,14 @@ export const lambdaHandler = async ( ); } + if (orderFromOrderUid.correlation_id === correlationId) { + console.info(name, "Duplicate request detected based on correlation ID", { + orderUid, + correlationId, + }); + return createFhirResponse(200, task); + } + try { await orderService.updateOrderStatusAndResultStatus( orderUid,