update evaluate cache to use a smaller key (#2939)

* update evaluate cache to use a smaller key

* [pre-commit.ci] auto fixes from pre-commit hooks

* use const instead of let

Co-authored-by: Mary McGrath <[email protected]>

* update clear-local to call seed scripts docker compose

* point dockerfile to new version of fhir converter

* update save to look at fhirBundle.id instead of first entry

* update key and use map instead of object

* fix tests to match correct ID

* test: update snapshots

* Update containers/ecr-viewer/src/app/view-data/utils/evaluate.ts

Co-authored-by: Mary McGrath <[email protected]>

* simplify evaluate logic

* revert change to clear-local

* Revert "point dockerfile to new version of fhir converter"

This reverts commit 54b5979.

* Revert "update save to look at fhirBundle.id instead of first entry"

This reverts commit 2eb853b.

* Revert "test: update snapshots"

This reverts commit 81f49a4.

* Revert "fix tests to match correct ID"

This reverts commit c1a3369.

* use keyword bundle

* [pre-commit.ci] auto fixes from pre-commit hooks

* simplify key

* replace beforeEach clear evaluate cache with a global one

* change to use spy

* fix lab info test by using beforeAll

* revert change to package-lock

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mary McGrath <[email protected]>

containers/ecr-viewer/jest.setup.ts

@@ -3,6 +3,7 @@ import { toHaveNoViolations } from "jest-axe";
 import * as matchers from "jest-extended";
 import { TextEncoder } from "util";
 import router from "next-router-mock";
+import { clearEvaluateCache } from "@/app/view-data/utils/evaluate";
 
 global.TextEncoder = TextEncoder;
 
@@ -21,3 +22,7 @@ jest.mock("next/navigation", () => ({
     };
   },
 }));
+
+beforeEach(() => {
+  clearEvaluateCache();
+});

containers/ecr-viewer/src/app/tests/components/Clinical.test.tsx

@@ -52,7 +52,7 @@ describe("Snapshot test for Procedures (Treatment Details)", () => {
         performedDateTime: "2022-06-24T12:50:00-04:00",
       },
       {
-        id: "b40f0081-4052-4971-3f3b-e3d9f5e1e44d",
+        id: "b40f0081-4052-4971-3f3b-e3d9f5e1e44e",
         code: {
           coding: [
             {

containers/ecr-viewer/src/app/tests/components/LabInfo.test.tsx

@@ -16,16 +16,19 @@ const mappings = loadYamlConfig();
 
 describe("LabInfo", () => {
   describe("when labResults is LabReportElementData[]", () => {
-    const labinfoOrg = evaluateLabInfoData(
-      BundleLab as unknown as Bundle,
-      evaluate(BundleLab, mappings["diagnosticReports"]),
-      mappings,
-    ) as LabReportElementData[];
+    let labInfoJsx: React.ReactElement;
+    beforeAll(() => {
+      const labinfoOrg = evaluateLabInfoData(
+        BundleLab as unknown as Bundle,
+        evaluate(BundleLab, mappings["diagnosticReports"]),
+        mappings,
+      ) as LabReportElementData[];
 
-    // Empty out one of the lab names for testing
-    labinfoOrg[0].organizationDisplayDataProps[0].value = "";
+      // Empty out one of the lab names for testing
+      labinfoOrg[0].organizationDisplayDataProps[0].value = "";
 
-    const labInfoJsx = <LabInfo labResults={labinfoOrg} />;
+      labInfoJsx = <LabInfo labResults={labinfoOrg} />;
+    });
     it("all should be collapsed by default", () => {
       render(labInfoJsx);
 
@@ -92,18 +95,21 @@ describe("LabInfo", () => {
         });
     });
     it("should match snapshot test", () => {
-      const container = render(labInfoJsx).container;
+      const { container } = render(labInfoJsx);
       expect(container).toMatchSnapshot();
     });
   });
 
   describe("when labResults is DisplayDataProps[]", () => {
-    const labinfo = evaluateLabInfoData(
-      BundleLabNoLabIds as unknown as Bundle,
-      evaluate(BundleLabNoLabIds, mappings["diagnosticReports"]),
-      mappings,
-    );
-    const labInfoJsx = <LabInfo labResults={labinfo} />;
+    let labInfoJsx: React.ReactElement;
+    beforeAll(() => {
+      const labinfo = evaluateLabInfoData(
+        BundleLabNoLabIds as unknown as Bundle,
+        evaluate(BundleLabNoLabIds, mappings["diagnosticReports"]),
+        mappings,
+      );
+      labInfoJsx = <LabInfo labResults={labinfo} />;
+    });
     it("should be collapsed by default", () => {
       render(labInfoJsx);
 
@@ -119,7 +125,7 @@ describe("LabInfo", () => {
         });
     });
     it("should match snapshot test", () => {
-      const container = render(labInfoJsx).container;
+      const { container } = render(labInfoJsx);
       expect(container).toMatchSnapshot();
     });
   });

containers/ecr-viewer/src/app/view-data/utils/evaluate.test.ts

@@ -1,30 +1,88 @@
 import { evaluate } from "@/app/view-data/utils/evaluate";
-import { evaluate as fhirPathEvaluate } from "fhirpath";
-
-jest.mock("fhirpath", () => ({
-  evaluate: jest.fn(),
-}));
 
 describe("evaluate", () => {
+  let fhirPathEvaluateSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    fhirPathEvaluateSpy = jest.spyOn(require("fhirpath"), "evaluate");
+  });
+
   afterEach(() => {
-    // Clear the mock implementation and calls after each test
     jest.clearAllMocks();
+    fhirPathEvaluateSpy.mockRestore();
   });
+
   it("fhirpath should be called 1 time when 1 call is made ", () => {
     evaluate({ id: "1234" }, "id");
 
-    expect(fhirPathEvaluate).toHaveBeenCalledOnce();
+    expect(fhirPathEvaluateSpy).toHaveBeenCalledExactlyOnceWith(
+      { id: "1234" },
+      "id",
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+    );
   });
   it("should call fhirpath.evaluate 1 time when the same call is made 2 times", () => {
     evaluate({ id: "2345" }, "id");
     evaluate({ id: "2345" }, "id");
 
-    expect(fhirPathEvaluate).toHaveBeenCalledOnce();
+    expect(fhirPathEvaluateSpy).toHaveBeenCalledExactlyOnceWith(
+      { id: "2345" },
+      "id",
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+    );
   });
   it("should call fhirpath.evaluate 2 time when the context is different", () => {
     evaluate({ id: "%id" }, "id", { id: 1 });
     evaluate({ id: "%id" }, "id", { id: 2 });
 
-    expect(fhirPathEvaluate).toHaveBeenCalledTimes(2);
+    expect(fhirPathEvaluateSpy).toHaveBeenCalledTimes(2);
+    expect(fhirPathEvaluateSpy).toHaveBeenNthCalledWith(
+      1,
+      { id: "%id" },
+      "id",
+      { id: 1 },
+      undefined,
+      undefined,
+    );
+    expect(fhirPathEvaluateSpy).toHaveBeenNthCalledWith(
+      2,
+      { id: "%id" },
+      "id",
+      { id: 2 },
+      undefined,
+      undefined,
+    );
+  });
+
+  it("should call once if resource type is bundle", () => {
+    evaluate({ resourceType: "Bundle" }, "name");
+    evaluate({ resourceType: "Bundle" }, "name");
+
+    expect(fhirPathEvaluateSpy).toHaveBeenCalledExactlyOnceWith(
+      { resourceType: "Bundle" },
+      "name",
+      undefined,
+      undefined,
+      undefined,
+    );
+  });
+
+  it("should call once if id is the same", () => {
+    evaluate({ id: 1234 }, "name");
+    evaluate({ id: 1234, resourceType: "Observation" }, "name");
+
+    expect(fhirPathEvaluateSpy).toHaveBeenCalledExactlyOnceWith(
+      { id: 1234 },
+      "name",
+      undefined,
+      undefined,
+      undefined,
+    );
   });
 });

containers/ecr-viewer/src/app/view-data/utils/evaluate.ts

@@ -6,7 +6,7 @@ import {
   UserInvocationTable,
 } from "fhirpath";
 
-let cache: { [key: string]: any } = {};
+const evaluateCache: Map<string, any> = new Map();
 
 /**
  * Evaluates a FHIRPath expression on the provided FHIR data.
@@ -31,15 +31,24 @@ export const evaluate = (
     userInvocationTable?: UserInvocationTable;
   },
 ): any[] => {
-  let key =
-    JSON.stringify(fhirData) +
-    ",,,,," +
-    JSON.stringify(context) +
-    ",,,,," +
-    JSON.stringify(path);
-  if (cache.hasOwnProperty(key)) {
-    return cache[key];
+  // Since the bundle does not have an ID, prefer to just use "bundle" instead
+  let fhirDataIdentifier: string =
+    (fhirData?.resourceType === "Bundle" ? "bundle" : fhirData?.id) ??
+    JSON.stringify(fhirData);
+  const key =
+    fhirDataIdentifier + JSON.stringify(context) + JSON.stringify(path);
+  if (!evaluateCache.has(key)) {
+    evaluateCache.set(
+      key,
+      fhirPathEvaluate(fhirData, path, context, model, options),
+    );
   }
-  cache[key] = fhirPathEvaluate(fhirData, path, context, model, options);
-  return cache[key];
+  return evaluateCache.get(key);
+};
+
+/**
+ * Reset the evaluate cache map
+ */
+export const clearEvaluateCache = () => {
+  evaluateCache.clear();
 };