Merge branch 'bugfix/LF-2863/updated-hasValue-function' into 'master'

Fixed incorrect behavior of the hasValue() function See merge request lfor/fhirpath.js!7
HL7 · Mar 13, 2024 · 5428ef8 · 5428ef8
2 parents b96acc8 + 8fd9f93
commit 5428ef8
Show file tree

Hide file tree

Showing 16 changed files with 193 additions and 121 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,11 @@
 This log documents significant changes for each release.  This project follows
 [Semantic Versioning](http://semver.org/).
 
+## [3.10.4] - 2024-03-13
+### Fixed
+- hasValue() function previously only checked the data type of an input
+  single-element collection, but not the existence of a value.
+
 ## [3.10.3] - 2024-03-12
 ### Fixed
 - Functions `as(<type specifier>)`, `is(<type specifier>)`,

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.10.3",
+  "version": "3.10.4",
   "description": "A FHIRPath engine",
   "main": "src/fhirpath.js",
   "dependencies": {

diff --git a/src/misc.js b/src/misc.js
@@ -5,7 +5,7 @@
 var util = require("./utilities");
 var types = require("./types");
 
-const { FP_Quantity } = types;
+const { FP_Quantity, TypeInfo } = types;
 
 var engine = {};
 
@@ -283,7 +283,7 @@ engine.singleton = function (coll, type) {
 /**
  * Checks whether a primitve value is present
  */
-const fhirPrimitives = new Set();
+const primitives = new Set();
 // IE11 probably doesn't support `new Set(iterable)`
 [
   "instant",
@@ -305,33 +305,18 @@ const fhirPrimitives = new Set();
   "oid",
   "uuid",
   "canonical",
-  "url"
-].forEach(i => fhirPrimitives.add(i));
+  "url",
+  "Integer",
+  "Decimal",
+  "String",
+  "Date",
+  "DateTime",
+  "Time"
+].forEach(i => primitives.add(i));
 
 engine.hasValueFn = function(coll) {
-  let model = this.model;
-
-  if (coll.length === 1){
-    if(model){
-      return [fhirPrimitives.has(coll[0].path)];
-    } else {
-      return [isPrimitiveDefault(util.valData(coll[0]))];
-    }
-  } else {
-    return [false];
-  }
+  return coll.length === 1 && util.valData(coll[0]) != null
+    && primitives.has(TypeInfo.fromValue(coll[0]).name);
 };
 
-function isPrimitiveDefault(data){
-  switch (typeof data){
-    case 'string':
-    case 'number':
-    case 'boolean':
-    // case 'bigint':
-      return true;
-    default:
-      return false;
-  }
-}
-
 module.exports = engine;
diff --git a/test/benchmark/common-benchmark-utils.js b/test/benchmark/common-benchmark-utils.js
@@ -1,10 +1,11 @@
 /**
- * Prepares a suite for a FHIRPath expression with 4 cases:
- *  1. big items using evaluate
- *  2. big items using compile
- *  3. small items using evaluate
- *  4. small items using compile
+ * Prepares 4 suites for a FHIRPath expression with 1 cases each:
+ *  1. big items using evaluate()
+ *  2. big items using compile()
+ *  3. small items using evaluate()
+ *  4. small items using compile()
  * @param {Object} desc - suite description.
+ * @param {string} desc.filename - output filename.
  * @param {string} desc.expression - FHIRPath expression.
  * @param {Array} desc.bigItems - array of big items to use in expression as
  *  "%items" variable.
@@ -15,9 +16,10 @@
  * @param {Array} desc.smallItemsCopy - copy of desc.smallItems in case
  *  expression uses it as "%itemsCopy".
  * @param {Object} desc.options - command line options. See benchmark.js.
- * @return an object describing a suite with 4 cases for use in benchmark.js.
+ * @param {Object} desc.options.compileOnly - skip test suites that uses evaluate().
+ * @return an array of objects describing suites with cases for use in benchmark.js.
  */
-function createSuiteForExpression(desc) {
+function createSuitesForExpression(desc) {
   const collectionOfBigItems = desc.bigItems;
   const collectionOfBigItemsCopy = desc.bigItemsCopy;
   const numberOfBigItems = collectionOfBigItems.length;
@@ -27,49 +29,61 @@ function createSuiteForExpression(desc) {
   const collectionOfSmallItemsCopy = desc.smallItemsCopy;
   const numberOfSmallItems = collectionOfSmallItems.length;
 
-  return {
+  return [...(options.compileOnly ? [] : [{
     name: desc.name,
-    filename: desc.filename,
+    filename: desc.filename + '-big-items-evaluate',
+    expression: desc.expression,
+    cases: [
+      {
+        name: `${numberOfBigItems} big items using evaluate()`,
+        testFunction: (fhirpath, model) => {
+          fhirpath.evaluate({}, desc.expression, {
+            items: collectionOfBigItems,
+            itemsCopy: collectionOfBigItemsCopy
+          }, model);
+        }
+      }
+    ]
+  },{
+    name: desc.name,
+    filename: desc.filename + '-big-items-evaluate',
     expression: desc.expression,
     cases: [
-      ...(options.compileOnly
-        ? []
-        : [{
-          name: `${numberOfBigItems} big items using evaluate()`,
-          testFunction: (fhirpath, model) => {
-            fhirpath.evaluate({}, desc.expression, {
-              items: collectionOfBigItems,
-              itemsCopy: collectionOfBigItemsCopy
-            }, model);
-          }
-        }]
-      ),
       {
         name: `${numberOfBigItems} big items using compile()`,
         testFunction: (fhirpath, model, compiledFn) => {
           compiledFn({}, { items: collectionOfBigItems, itemsCopy: collectionOfBigItemsCopy });
         }
-      },
-      ...(options.compileOnly
-        ? []
-        : [{
-          name: `${numberOfSmallItems} small items using evaluate()`,
-          testFunction: (fhirpath, model) => {
-            fhirpath.evaluate({}, desc.expression, { items: collectionOfSmallItems, itemsCopy: collectionOfSmallItemsCopy }, model);
-          }
-        }]
-      ),
+      }
+    ]
+  }]),{
+    name: desc.name,
+    filename: desc.filename + '-small-items-evaluate',
+    expression: desc.expression,
+    cases: [
+      {
+        name: `${numberOfSmallItems} small items using evaluate()`,
+        testFunction: (fhirpath, model) => {
+          fhirpath.evaluate({}, desc.expression, { items: collectionOfSmallItems, itemsCopy: collectionOfSmallItemsCopy }, model);
+        }
+      }
+    ]
+  },{
+    name: desc.name,
+    filename: desc.filename + '-small-items-compile',
+    expression: desc.expression,
+    cases: [
       {
         name: `${numberOfSmallItems} small items using compile()`,
         testFunction: (fhirpath, model, compiledFn) => {
           compiledFn({}, { items: collectionOfSmallItems, itemsCopy: collectionOfSmallItemsCopy });
         }
       }
     ]
-  };
+  }];
 
 }
 
 module.exports = {
-  createSuiteForExpression
+  createSuitesForExpression: createSuitesForExpression
 }
diff --git a/test/benchmark/contains.js b/test/benchmark/contains.js
@@ -14,30 +14,49 @@ module.exports = ({
   const numberOfSmallItems = current_fhirpath.evaluate({}, '%items.count()', { items: smallItems }, current_r4_model)[0];
 
   const expression = '%items contains %lastOfItemsCopy';
+  const name = 'Checking if a list of items contains some item';
 
   return [{
-    name: `Checking if a list of items contains some item`,
-    filename: 'contains',
+    name,
+    filename: 'contains-big-items-evaluate',
     expression,
     cases: [
       {
         name: `${numberOfBigItems} big items using evaluate()`,
         testFunction: (fhirpath, model) => {
           fhirpath.evaluate({}, expression, { items: bigItems, lastOfItemsCopy: lastOfBigItemsCopy }, model);
         }
-      },
+      }
+    ]
+  },{
+    name,
+    filename: 'contains-big-items-compile',
+    expression,
+    cases: [
       {
         name: `${numberOfBigItems} big items using compile()`,
         testFunction: (fhirpath, model, compiledFn) => {
           compiledFn({}, { items: bigItems, lastOfItemsCopy: lastOfBigItemsCopy });
         }
-      },
+      }
+    ]
+  },{
+    name,
+    filename: 'contains-small-items-evaluate',
+    expression,
+    cases: [
       {
         name: `${numberOfSmallItems} small items using evaluate()`,
         testFunction: (fhirpath, model) => {
           fhirpath.evaluate({}, expression, { items: smallItems, lastOfItemsCopy: lastOfSmallItemsCopy }, model);
         }
-      },
+      }
+    ]
+  },{
+    name,
+    filename: 'contains-small-items-compile',
+    expression,
+    cases: [
       {
         name: `${numberOfSmallItems} small items using compile()`,
         testFunction: (fhirpath, model, compiledFn) => {

diff --git a/test/benchmark/descendants.js b/test/benchmark/descendants.js
@@ -12,26 +12,30 @@ module.exports = ({
   const numberOfItems = current_fhirpath.evaluate(patientExampleCopy1,'Patient.descendants().count()', {},  current_r4_model);
   const expression = 'Patient.descendants()';
 
-  return [{
-    name: `Getting ${numberOfItems} descendants`,
-    filename: 'descendants',
-    expression,
-    cases: [
-      ...(options.compileOnly
-        ? []
-        : [{
-          name: `using evaluate()`,
+  return [
+    ...(options.compileOnly
+      ? []
+      : [{
+        name: `Getting ${numberOfItems} descendants using evaluate()`,
+        filename: 'descendants-evaluate',
+        expression,
+        cases: [{
+          name: '',
           testFunction: (fhirpath, model) => {
             fhirpath.evaluate(fhirpath === current_fhirpath ? patientExampleCopy1 : PatientExampleCopy2, expression, {}, model);
           }
-        }]),
-      {
-        name: `using compile()`,
+        }]
+      }]),
+    {
+      name: `Getting ${numberOfItems} descendants using compile()`,
+      filename: 'descendants-compile',
+      expression,
+      cases: [{
+        name: '',
         testFunction: (fhirpath, model, compiledFn) => {
           compiledFn(fhirpath === current_fhirpath ? patientExampleCopy1 : PatientExampleCopy2, {});
         }
-      }
-    ]
-  }];
+      }]
+    }];
 
 }
diff --git a/test/benchmark/distinct.js b/test/benchmark/distinct.js
@@ -1,5 +1,5 @@
 const { maxCollSizeForDeepEqual } = require('../../src/deep-equal');
-const { createSuiteForExpression } = require('./common-benchmark-utils');
+const { createSuitesForExpression } = require('./common-benchmark-utils');
 
 module.exports = ({
                     bigItems,
@@ -22,6 +22,6 @@ module.exports = ({
     bigItems: bigItems.slice(-maxCollSizeForDeepEqual),
     smallItems: smallItems.slice(-maxCollSizeForDeepEqual),
     options
-  }].map(createSuiteForExpression);
+  }].map(createSuitesForExpression);
 
 }
diff --git a/test/benchmark/exclude.js b/test/benchmark/exclude.js
@@ -1,6 +1,6 @@
 const _ = require('lodash');
 const { maxCollSizeForDeepEqual } = require('../../src/deep-equal');
-const { createSuiteForExpression } = require('./common-benchmark-utils');
+const { createSuitesForExpression } = require('./common-benchmark-utils');
 
 module.exports = ({
                     bigItems,
@@ -30,6 +30,6 @@ module.exports = ({
     smallItems: smallItems.slice(-smallCollectionLength),
     smallItemsCopy: smallItemsCopy.slice(-smallCollectionLength),
     options
-  }].map(createSuiteForExpression);
+  }].map(createSuitesForExpression);
 
 }
diff --git a/test/benchmark/intersect.js b/test/benchmark/intersect.js
@@ -1,6 +1,5 @@
-const _ = require('lodash');
 const { maxCollSizeForDeepEqual } = require('../../src/deep-equal');
-const { createSuiteForExpression } = require('./common-benchmark-utils');
+const { createSuitesForExpression } = require('./common-benchmark-utils');
 
 module.exports = ({
                     bigItems,
@@ -30,6 +29,6 @@ module.exports = ({
     smallItems: smallItems.slice(-smallCollectionLength),
     smallItemsCopy: smallItemsCopy.slice(-smallCollectionLength),
     options
-  }].map(createSuiteForExpression);
+  }].map(createSuitesForExpression);
 
 }