Issue/1611 stale async expression cache

packages/smart-forms-renderer/src/test/fhirpath.test.ts

@@ -20,10 +20,12 @@
 
 import {
   addEmptyXFhirQueryVariablesToFhirPathContext,
+  cacheTerminologyResult,
   createFhirPathContext,
   evaluateLinkIdVariables,
   evaluateQuestionnaireLevelVariables,
-  handleFhirPathResult
+  handleFhirPathResult,
+  isExpressionCached
 } from '../utils/fhirpath';
 import {
   BMICalculationExistingFhirPathContext,
@@ -247,3 +249,110 @@ describe('handleFhirPathResult', () => {
     expect(result).toEqual(['valueA', 'valueB']);
   });
 });
+
+describe('isExpressionCached', () => {
+  it('returns false for expression containing %', () => {
+    expect(
+      isExpressionCached('%resource.memberOf(url)', { '%resource.memberOf(url)': [true] })
+    ).toBe(false);
+  });
+
+  it('returns true when expression-only key is in cache', () => {
+    expect(isExpressionCached('memberOf(url)', { 'memberOf(url)': [true] })).toBe(true);
+  });
+
+  it('returns false when expression is not in cache', () => {
+    expect(isExpressionCached('memberOf(url)', {})).toBe(false);
+  });
+
+  it('returns true when compound key (expression + focusNode) is in cache', () => {
+    const focusNode = { answer: [{ valueString: 'A' }] };
+    const key = `memberOf(url)|${JSON.stringify(focusNode)}`;
+    expect(isExpressionCached('memberOf(url)', { [key]: [true] }, focusNode)).toBe(true);
+  });
+
+  it('returns false when focusNode differs from the cached one', () => {
+    const cachedFocus = { answer: [{ valueString: 'A' }] };
+    const newFocus = { answer: [{ valueString: 'B' }] };
+    const key = `memberOf(url)|${JSON.stringify(cachedFocus)}`;
+    expect(isExpressionCached('memberOf(url)', { [key]: [true] }, newFocus)).toBe(false);
+  });
+
+  it('misses cache when focusNode is provided but only the expression-only key exists', () => {
+    const focusNode = { answer: [{ valueString: 'A' }] };
+    expect(isExpressionCached('memberOf(url)', { 'memberOf(url)': [true] }, focusNode)).toBe(false);
+  });
+});
+
+describe('cacheTerminologyResult', () => {
+  it('skips caching for expressions with %', () => {
+    const cache: Record<string, any> = {};
+    cacheTerminologyResult('%resource.memberOf(url)', [true], cache);
+    expect(cache).toEqual({});
+  });
+
+  it('stores result under expression key when no focusNode', () => {
+    const cache: Record<string, any> = {};
+    cacheTerminologyResult('memberOf(url)', [true], cache);
+    expect(cache).toEqual({ 'memberOf(url)': [true] });
+  });
+
+  it('stores result under compound key when focusNode is provided', () => {
+    const cache: Record<string, any> = {};
+    const focusNode = { answer: [{ valueString: 'A' }] };
+    cacheTerminologyResult('memberOf(url)', [true], cache, focusNode);
+    const expectedKey = `memberOf(url)|${JSON.stringify(focusNode)}`;
+    expect(cache).toEqual({ [expectedKey]: [true] });
+  });
+});
+
+describe('evaluateLinkIdVariables - stale cache prevention', () => {
+  const terminologyServerUrl = 'https://example.com/terminology/fhir';
+  const expression = 'answer.value';
+  const variablesFhirPath: Record<string, any[]> = {
+    'item-link': [{ name: 'myVar', language: 'text/fhirpath', expression }]
+  };
+
+  it('re-evaluates when qrItem changes, preventing stale async cache results', async () => {
+    const qrItemA = { linkId: 'item-link', answer: [{ valueString: 'A' }] };
+    const qrItemB = { linkId: 'item-link', answer: [{ valueString: 'B' }] };
+
+    // Simulate a previously cached async terminology result for qrItemA
+    const cacheKeyA = `${expression}|${JSON.stringify(qrItemA)}`;
+    const terminologyCache: Record<string, any> = { [cacheKeyA]: ['A'] };
+
+    // Evaluate with qrItemB - compound key is different, so cache should miss
+    const result = await evaluateLinkIdVariables(
+      'item-link',
+      variablesFhirPath,
+      { myVar: ['A'] }, // stale context from previous eval with qrItemA
+      terminologyCache,
+      terminologyServerUrl,
+      qrItemB
+    );
+
+    // Should re-evaluate with qrItemB's answer, not return stale 'A'
+    expect(result.fhirPathContext.myVar).toEqual(['B']);
+  });
+
+  it('skips re-evaluation when qrItem is unchanged (valid cache hit)', async () => {
+    const qrItem = { linkId: 'item-link', answer: [{ valueString: 'A' }] };
+
+    // Simulate a previously cached async terminology result for this qrItem
+    const cacheKey = `${expression}|${JSON.stringify(qrItem)}`;
+    const terminologyCache: Record<string, any> = { [cacheKey]: ['A'] };
+
+    // Evaluate with the same qrItem - compound key matches, should hit cache
+    const result = await evaluateLinkIdVariables(
+      'item-link',
+      variablesFhirPath,
+      { myVar: ['A'] }, // context already has correct value
+      terminologyCache,
+      terminologyServerUrl,
+      qrItem
+    );
+
+    // Cache hit - fhirPathContext keeps the existing value
+    expect(result.fhirPathContext.myVar).toEqual(['A']);
+  });
+});

packages/smart-forms-renderer/src/utils/fhirpath.ts

@@ -267,13 +267,14 @@ export async function evaluateLinkIdVariables(
 
   for (const variable of linkIdVariables) {
     if (variable.expression && variable.name) {
-      if (isExpressionCached(variable.expression, fhirPathTerminologyCache)) {
+      const focusNode = qrItem ?? {};
+      if (isExpressionCached(variable.expression, fhirPathTerminologyCache, focusNode)) {
         continue;
       }
 
       try {
         const fhirPathResult = fhirpath.evaluate(
-          qrItem ?? {},
+          focusNode,
           {
             base: 'QuestionnaireResponse.item',
             expression: variable.expression
@@ -288,9 +289,10 @@ export async function evaluateLinkIdVariables(
         const result = await handleFhirPathResult(fhirPathResult);
         fhirPathContext[`${variable.name}`] = result;
 
-        // If fhirPathResult is an async terminology call, cache the result
+        // If fhirPathResult is an async terminology call, cache the result keyed by both
+        // expression and focus node so that a changed answer invalidates the cached result
         if (fhirPathResult instanceof Promise) {
-          cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache);
+          cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache, focusNode);
         }
       } catch (e) {
         console.warn(e.message, `LinkId: ${linkId}\nExpression: ${variable.expression}`);
@@ -318,7 +320,7 @@ export async function evaluateQuestionnaireLevelVariables(
 
   for (const variable of questionnaireLevelVariables) {
     if (variable.expression) {
-      if (isExpressionCached(variable.expression, fhirPathTerminologyCache)) {
+      if (isExpressionCached(variable.expression, fhirPathTerminologyCache, resource)) {
         continue;
       }
 
@@ -340,9 +342,10 @@ export async function evaluateQuestionnaireLevelVariables(
         const result = await handleFhirPathResult(fhirPathResult);
         fhirPathContext[`${variable.name}`] = result;
 
-        // If fhirPathResult is an async terminology call, cache the result
+        // If fhirPathResult is an async terminology call, cache the result keyed by both
+        // expression and focus node so that a changed response invalidates the cached result
         if (fhirPathResult instanceof Promise) {
-          cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache);
+          cacheTerminologyResult(variable.expression, result, fhirPathTerminologyCache, resource);
         }
       } catch (e) {
         console.warn(e.message, `Questionnaire-level\nExpression: ${variable.expression}`);
@@ -368,40 +371,46 @@ export async function handleFhirPathResult(result: any[] | Promise<any[]>) {
  * Determines whether a FHIRPath expression result is cached.
  *
  * Expressions containing variables (e.g. `%something`) are never considered cached, because we can never know what a variable resolves to at runtime.
+ * When a `focusNode` is provided, it is included in the cache key so that results are not reused across different inputs (e.g. different `qrItem` values for `memberOf()` calls).
  *
  * @param {string} expression - The FHIRPath expression to check.
  * @param {Record<string, any>} fhirPathTerminologyCache - Object storing cached expression results.
+ * @param {unknown} [focusNode] - Optional focus node passed to fhirpath.evaluate(). When provided it is serialised as part of the cache key.
  * @returns {boolean} `true` if the expression is cached and contains no variables; otherwise `false`.
  */
 export function isExpressionCached(
   expression: string,
-  fhirPathTerminologyCache: Record<string, any>
+  fhirPathTerminologyCache: Record<string, any>,
+  focusNode?: unknown
 ): boolean {
   // Expressions with variables are never cached
   if (expression.includes('%')) {
     return false;
   }
-
+  const key = focusNode !== undefined ? `${expression}|${JSON.stringify(focusNode)}` : expression;
   // Check if expression exists in cache
-  return Object.prototype.hasOwnProperty.call(fhirPathTerminologyCache, expression);
+  return Object.prototype.hasOwnProperty.call(fhirPathTerminologyCache, key);
 }
 
 /**
  * Caches the result of a FHIRPath evaluation if it was an asynchronous terminology call and the expression contains no variables (e.g. `%something`).
+ * When a `focusNode` is provided, it is included in the cache key to differentiate results for different inputs.
  *
  * @param {string} expression - The FHIRPath expression that was evaluated.
  * @param {any} result - The evaluated result of the expression with async resolved.
  * @param {Record<string, any>} fhirPathTerminologyCache - The cache object to store results.
+ * @param {unknown} [focusNode] - Optional focus node passed to fhirpath.evaluate(). When provided it is serialised as part of the cache key.
  */
 export function cacheTerminologyResult(
   expression: string,
   result: any,
-  fhirPathTerminologyCache: Record<string, any>
+  fhirPathTerminologyCache: Record<string, any>,
+  focusNode?: unknown
 ) {
   // Skip caching for expressions with variables
   if (expression.includes('%')) {
     return;
   }
-
-  fhirPathTerminologyCache[expression] = result;
+  const key = focusNode !== undefined ? `${expression}|${JSON.stringify(focusNode)}` : expression;
+  fhirPathTerminologyCache[key] = result;
 }