Merge branch 'bugfix/LF-3005/resource-in-environment-var' into 'master'

yuriy-sedinkin · yuriy-sedinkin · commit 5d3e04fe47bc · 2024-05-15T17:50:26.000Z
Fixed an issue with evaluating an expression for a resource passed through an environment variable

See merge request lfor/fhirpath.js!12
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,12 @@
 This log documents significant changes for each release.  This project follows
 [Semantic Versioning](http://semver.org/).
 
+## [3.13.2] - 2024-05-15
+### Fixed
+- an issue with evaluating an expression for a resource passed through an
+  environment variable.
+- an issue with "statusShift" during performance tests.
+
 ## [3.13.1] - 2024-04-25
 ### Fixed
 - Added flag 'u' for regular expressions in the specification's `matches` and
diff --git a/fhir-context/dstu2/index.js b/fhir-context/dstu2/index.js
@@ -1,3 +1,4 @@
+const updateWithGeneratedData = require('../general-additions');
 /**
  *  Exports the FHIR model data for DSTU2.  This is an internal structure that
  *  will likely evolve as more FHIR specific processing is added.
@@ -24,10 +25,7 @@ const modelInfo = {
   path2Type: require('./path2Type.json')
 };
 
-// Generate a set of available data types
-modelInfo.availableTypes = new Set();
-// IE11 probably doesn't support `new Set(iterable)`
-Object.keys(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
-Object.values(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
+// Update with generated data
+updateWithGeneratedData(modelInfo)
 
 module.exports = modelInfo;
diff --git a/fhir-context/general-additions.js b/fhir-context/general-additions.js
@@ -0,0 +1,17 @@
+// Update the model with generated data
+module.exports = (modelInfo) => {
+  // Generate a set of available data types
+  modelInfo.availableTypes = new Set();
+  // IE11 probably doesn't support `new Set(iterable)`
+  Object.keys(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
+  Object.values(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
+
+  // Generate a hash map to map paths to data types excluding "BackboneElement" and "Element".
+  modelInfo.path2TypeWithoutElements = {};
+  for(let i in modelInfo.path2Type) {
+    if (modelInfo.path2Type[i] === 'Element' || modelInfo.path2Type[i] === 'BackboneElement') {
+      continue;
+    }
+    modelInfo.path2TypeWithoutElements[i] = modelInfo.path2Type[i];
+  }
+};
diff --git a/fhir-context/r4/index.js b/fhir-context/r4/index.js
@@ -1,3 +1,5 @@
+const updateWithGeneratedData = require('../general-additions');
+
 /**
  *  Exports the FHIR model data for R4.  This is an internal structure that
  *  will likely evolve as more FHIR specific processing is added.
@@ -24,10 +26,7 @@ const modelInfo = {
   path2Type: require('./path2Type.json')
 };
 
-// Generate a set of available data types
-modelInfo.availableTypes = new Set();
-// IE11 probably doesn't support `new Set(iterable)`
-Object.keys(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
-Object.values(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
+// Update with generated data
+updateWithGeneratedData(modelInfo)
 
 module.exports = modelInfo;
diff --git a/fhir-context/r5/index.js b/fhir-context/r5/index.js
@@ -1,3 +1,4 @@
+const updateWithGeneratedData = require('../general-additions');
 /**
  *  Exports the FHIR model data for R5.  This is an internal structure that
  *  will likely evolve as more FHIR specific processing is added.
@@ -24,10 +25,7 @@ const modelInfo = {
   path2Type: require('./path2Type.json')
 };
 
-// Generate a set of available data types
-modelInfo.availableTypes = new Set();
-// IE11 probably doesn't support `new Set(iterable)`
-Object.keys(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
-Object.values(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
+// Update with generated data
+updateWithGeneratedData(modelInfo)
 
 module.exports = modelInfo;
diff --git a/fhir-context/stu3/index.js b/fhir-context/stu3/index.js
@@ -1,3 +1,4 @@
+const updateWithGeneratedData = require('../general-additions');
 /**
  *  Exports the FHIR model data for STU3.  This is an internal structure that
  *  will likely evolve as more FHIR specific processing is added.
@@ -24,11 +25,8 @@ const modelInfo = {
   path2Type: require('./path2Type.json')
 };
 
-// Generate a set of available data types
-modelInfo.availableTypes = new Set();
-// IE11 probably doesn't support `new Set(iterable)`
-Object.keys(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
-Object.values(modelInfo.type2Parent).forEach(i => modelInfo.availableTypes.add(i));
+// Update with generated data
+updateWithGeneratedData(modelInfo)
 
 module.exports = modelInfo;
 
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "fhirpath",
-  "version": "3.13.1",
+  "version": "3.13.2",
   "description": "A FHIRPath engine",
   "main": "src/fhirpath.js",
   "dependencies": {
@@ -15,7 +15,7 @@
     "@babel/eslint-parser": "^7.17.0",
     "@babel/preset-env": "^7.16.11",
     "babel-loader": "^8.2.3",
-    "benny": "^3.7.1",
+    "benny": "github:caderek/benny#0ad058d3c7ef0b488a8fe9ae3519159fc7f36bb6",
     "bestzip": "^2.2.0",
     "copy-webpack-plugin": "^6.0.3",
     "cypress": "^13.7.2",
@@ -41,6 +41,7 @@
     "node": ">=8.9.0"
   },
   "scripts": {
+    "postinstall": "echo \"Building the Benny package based on a pull request which fixes an issue with 'statusShift'... \" && (cd node_modules/benny && npm i && npm run build > /dev/null) || echo \"Building the Benny package is completed.\"",
     "generateParser": "cd src/parser; rimraf ./generated/*; java -Xmx500M -cp \"../../antlr-4.9.3-complete.jar:$CLASSPATH\" org.antlr.v4.Tool -o generated -Dlanguage=JavaScript FHIRPath.g4; grunt updateParserRequirements",
     "build": "cd browser-build && webpack && rimraf fhirpath.zip && bestzip fhirpath.zip LICENSE.md fhirpath.min.js fhirpath.r5.min.js fhirpath.r4.min.js fhirpath.stu3.min.js fhirpath.dstu2.min.js && rimraf  LICENSE.md",
     "test:unit": "node --use_strict node_modules/.bin/jest && TZ=America/New_York node --use_strict node_modules/.bin/jest && TZ=Europe/Paris node --use_strict node_modules/.bin/jest",
diff --git a/src/fhirpath.js b/src/fhirpath.js
@@ -251,14 +251,44 @@ engine.ExternalConstantTerm = function(ctx, parentData, node) {
   var identifier = extConstant.children[0];
   var varName = engine.Identifier(ctx, parentData, identifier)[0];
 
-  var value = ctx.vars[varName];
-  if (!(varName in ctx.vars)) {
-    if (ctx.definedVars && varName in ctx.definedVars)
-      value = ctx.definedVars[varName];
-    else
-      throw new Error(
-        "Attempting to access an undefined environment variable: " + varName
-      );
+  var value;
+  // Check the user-defined environment variables first as the user can override
+  // the "context" variable like we do in unit tests. In this case, the user
+  // environment variable can replace the system environment variable in "processedVars".
+  if (varName in ctx.vars) {
+    // Restore the ResourceNodes for the top-level objects of the environment
+    // variables. The nested objects will be converted to ResourceNodes
+    // in the MemberInvocation method.
+    value = ctx.vars[varName];
+    if (Array.isArray(value)) {
+      value = value.map(
+        i => i?.__path__
+          ? makeResNode(i, i.__path__.path || null, null,
+            i.__path__.fhirNodeDataType || null)
+          : i?.resourceType
+            ? makeResNode(i, null, null)
+            : i );
+    } else {
+      value = value?.__path__
+        ? makeResNode(value, value.__path__.path || null, null,
+          value.__path__.fhirNodeDataType || null)
+        : value?.resourceType
+          ? makeResNode(value, null, null)
+          : value;
+    }
+    ctx.processedVars[varName] = value;
+    delete ctx.vars[varName];
+  } else if (varName in ctx.processedVars) {
+    // "processedVars" are variables with ready-to-use values that have already
+    // been converted to ResourceNodes if necessary.
+    value = ctx.processedVars[varName];
+  } else  if (ctx.definedVars && varName in ctx.definedVars) {
+    // "definedVars" are variables defined with the "defineVariable" function.
+    value = ctx.definedVars[varName];
+  } else {
+    throw new Error(
+      "Attempting to access an undefined environment variable: " + varName
+    );
   }
   // For convenience, we all variable values to be passed in without their array
   // wrapper.  However, when evaluating, we need to put the array back in.
@@ -656,27 +686,7 @@ function applyParsedPath(resource, parsedPath, context, model, options) {
   // Set up default standard variables, and allow override from the variables.
   // However, we'll keep our own copy of dataRoot for internal processing.
   let vars = {context: dataRoot, ucum: 'http://unitsofmeasure.org'};
-  // Restore the ResourceNodes for the top-level objects of the context
-  // variables. The nested objects will be converted to ResourceNodes
-  // in the MemberInvocation method.
-  if (context) {
-    context = Object.keys(context).reduce((restoredContext, key) => {
-      if (Array.isArray(context[key])) {
-        restoredContext[key] = context[key].map(
-          i => i?.__path__
-            ? makeResNode(i, i.__path__.path || null, null,
-              i.__path__.fhirNodeDataType || null)
-            : i );
-      } else {
-        restoredContext[key] = context[key]?.__path__
-          ? makeResNode(context[key], context[key].__path__.path || null, null,
-            context[key].__path__.fhirNodeDataType || null)
-          : context[key];
-      }
-      return restoredContext;
-    }, {});
-  }
-  let ctx = {dataRoot, vars: Object.assign(vars, context), model};
+  let ctx = {dataRoot, processedVars: vars, vars: context || {}, model};
   if (options.traceFn) {
     ctx.customTraceFn = options.traceFn;
   }
diff --git a/src/misc.js b/src/misc.js
@@ -55,7 +55,7 @@ engine.defineVariable = function (x, label, expr) {
   // Just in time initialization of definedVars
   if (!this.definedVars) this.definedVars = {};
 
-  if (Object.keys(this.vars).includes(label)) {
+  if (label in this.vars || label in this.processedVars) {
     throw new Error("Environment Variable %" + label + " already defined");
   }
 
diff --git a/src/types.js b/src/types.js
@@ -1315,23 +1315,26 @@ class ResourceNode {
    * @return {TypeInfo}
    */
   getTypeInfo() {
-    let result;
-
-    if (TypeInfo.model) {
-      if (/^System\.(.*)$/.test(this.fhirNodeDataType)) {
-        result = new TypeInfo({namespace: TypeInfo.System, name: RegExp.$1});
-      } else if (this.fhirNodeDataType) {
-        result = new TypeInfo({
-          namespace: TypeInfo.FHIR,
-          name: this.fhirNodeDataType
-        });
+    if (!this.typeInfo) {
+      let typeInfo;
+
+      if (TypeInfo.model) {
+        if (/^System\.(.*)$/.test(this.fhirNodeDataType)) {
+          typeInfo = new TypeInfo({namespace: TypeInfo.System, name: RegExp.$1});
+        } else if (this.fhirNodeDataType) {
+          typeInfo = new TypeInfo({
+            namespace: TypeInfo.FHIR,
+            name: this.fhirNodeDataType
+          });
+        }
       }
-    }
 
-    return result
-      // Resource object properties that are not defined in the model now have
-      // System.* data types:
-      || TypeInfo.createByValueInSystemNamespace(this.data);
+      this.typeInfo = typeInfo
+        // Resource object properties that are not defined in the model now have
+        // System.* data types:
+        || TypeInfo.createByValueInSystemNamespace(this.data);
+    }
+    return this.typeInfo;
   }
 
   toJSON() {
@@ -1350,24 +1353,27 @@ class ResourceNode {
    * @return {FP_Type|any}
    */
   convertData() {
-    var data = this.data;
-    const cls = TypeInfo.typeToClassWithCheckString[this.path];
-    if (cls) {
-      data = cls.checkString(data) || data;
-    } else if (TypeInfo.isType(this.path, 'Quantity')) {
-      if (data?.system === ucumSystemUrl) {
-        if (typeof data.value === 'number' && typeof data.code === 'string') {
-          if (data.comparator !== undefined)
-            throw new Error('Cannot convert a FHIR.Quantity that has a comparator');
-          data = new FP_Quantity(
-            data.value,
-            FP_Quantity.mapUCUMCodeToTimeUnits[data.code] || '\'' + data.code + '\''
-          );
+    if (!this.convertedData) {
+      var data = this.data;
+      const cls = TypeInfo.typeToClassWithCheckString[this.path];
+      if (cls) {
+        data = cls.checkString(data) || data;
+      } else if (TypeInfo.isType(this.path, 'Quantity')) {
+        if (data?.system === ucumSystemUrl) {
+          if (typeof data.value === 'number' && typeof data.code === 'string') {
+            if (data.comparator !== undefined)
+              throw new Error('Cannot convert a FHIR.Quantity that has a comparator');
+            data = new FP_Quantity(
+              data.value,
+              FP_Quantity.mapUCUMCodeToTimeUnits[data.code] || '\'' + data.code + '\''
+            );
+          }
         }
       }
-    }
 
-    return data;
+      this.convertedData = data;
+    }
+    return this.convertedData;
   }
 
 }
diff --git a/src/utilities.js b/src/utilities.js
@@ -162,8 +162,11 @@ util.makeChildResNodes = function(parentResNode, childProperty, model) {
     }
   }
 
-  const fhirNodeDataType = model && model.path2Type[childPath] || null;
-  childPath = fhirNodeDataType === 'BackboneElement' || fhirNodeDataType === 'Element' ? childPath : fhirNodeDataType || childPath;
+  let fhirNodeDataType = null;
+  if (model) {
+    fhirNodeDataType = model.path2Type[childPath] || null;
+    childPath = model.path2TypeWithoutElements[childPath] || childPath;
+  }
 
   let result;
   if (util.isSome(toAdd) || util.isSome(_toAdd)) {
diff --git a/test/api.test.js b/test/api.test.js
diff --git a/test/resources/questionnaire-response-example.json b/test/resources/questionnaire-response-example.json

Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ engine.defineVariable = function (x, label, expr) {`
`55`	`55`	`// Just in time initialization of definedVars`
`56`	`56`	`if (!this.definedVars) this.definedVars = {};`
`57`	`57`
`58`		`- if (Object.keys(this.vars).includes(label)) {`
	`58`	`+ if (label in this.vars \|\| label in this.processedVars) {`
`59`	`59`	`throw new Error("Environment Variable %" + label + " already defined");`
`60`	`60`	`}`
`61`	`61`