copilot feedback

Author: TSeelig

lambdas/src/lib/db/order-db.test.ts

@@ -1,23 +1,18 @@
-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();
 
 describe("OrderService", () => {
   let dbClient: any;
-  let commons: Pick<Commons, "logError">;
   let orderService: OrderService;
 
   beforeEach(() => {
     dbClient = {
       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",
         {

lambdas/src/lib/db/order-db.ts

@@ -18,10 +18,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<OrderResultSummary | null> {
@@ -45,7 +43,7 @@ export class OrderService {
       const result = await this.dbClient.query<OrderResultSummary, [string]>(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 +62,7 @@ export class OrderService {
       const result = await this.dbClient.query<OrderPatientReference, [string]>(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 +97,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,
       });

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", () => {

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<void> {
     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,

lambdas/src/order-result-lambda/init.ts

@@ -26,7 +26,7 @@ export function buildEnvironment(): Environment {
 
   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);

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,
     },
@@ -66,9 +58,6 @@ describe("result-status-lambda handler", () => {
   beforeEach(() => {
     jest.clearAllMocks();
 
-    mockLogInfo.mockReset();
-    mockLogDebug.mockReset();
-    mockLogError.mockReset();
     mockUpdateResultStatus.mockReset();
     mockRetrievePatientIdFromOrder.mockReset();
 
@@ -221,7 +210,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 +226,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 +247,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(

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,26 +15,26 @@ import { corsOptions } from "./cors-configuration";
 import { init } from "./init";
 import { resultStatusFHIRTaskSchema } from "./schemas";
 
-function parseAndValidateTask(body: string | null, commons: Commons): FHIRTask {
+function parseAndValidateTask(body: string | null): FHIRTask {
   let parsedTask: unknown;
 
   if (!body) {
-    commons.logError("result-status-lambda", "Missing request body");
+    console.error("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 });
+    console.error("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 });
+    console.error("result-status-lambda", "Task validation failed", { error: errorDetails });
     throw new Error(`Task validation failed: ${errorDetails}`);
   }
 
@@ -78,16 +77,16 @@ function extractOrderUidFromFHIRTask(task: FHIRTask): string {
 export const lambdaHandler = async (
   event: APIGatewayProxyEvent,
 ): Promise<APIGatewayProxyResult> => {
-  const { commons, resultService, orderService } = init();
-  commons.logInfo("result-status-lambda", "Received result status request", {
+  const { resultService, orderService } = init();
+  console.info("result-status-lambda", "Received result status request", {
     path: event.path,
     method: event.httpMethod,
   });
 
   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,31 +99,42 @@ 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("result-status-lambda", "Failed to extract identifiers from FHIR Task", {
       error,
     });
     return createFhirErrorResponse(400, "invalid", message, "error");
   }
 
+  let correlationId: string;
+
+  try {
+    correlationId = getCorrelationIdFromEventHeaders(event);
+  } catch (error) {
+    console.error("result-status-lambda", "Failed to extract correlation ID from request headers", {
+      error,
+    });
+    return createFhirErrorResponse(400, "invalid", "Invalid correlation ID in headers", "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", {
+    console.error("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 });
+    console.error("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", {
+    console.error("result-status-lambda", "Patient UID in Task does not match order record", {
       orderUID,
     });
     return createFhirErrorResponse(
@@ -135,25 +145,10 @@ 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("result-status-lambda", "Failed to update result status in database", {
       error,
       orderUID,
     });