Merge pull request #54 in LFOR/fhirpath.js from bugfix/LF-2635/hashObject-for-null-values to master

* commit '0c5fd085b741455de562fc37d0d477d4241e8c29':
  npm audit fix
  Resolved merge conflicts
  Fixed a typo
  Fixed issues with null values
  Reverted unnecessary changes and did minor refactoring
  Added a benchmark for the "contains" operator
  Changes as per review
  Added unit tests for the hashObject function
  Fix hash for null values #138

Author: yuriy-sedinkin

CHANGELOG.md

@@ -3,6 +3,19 @@
 This log documents significant changes for each release.  This project follows
 [Semantic Versioning](http://semver.org/).
 
+## [3.9.1] - 2024-01-22
+### Fixed
+- Fixed exception in the "hashObject" internal function when an object has
+  a property with the "null" value. This may affect functions that compare
+  objects:
+  intersect(), subsetOf(), repeat(), union(), distinct(), isDistinct().
+- Null values are excluded from the expression evaluation result.
+- Fixed an issue when evaluating an expression for a resource object with missed
+  values at the end of the array in a property.
+- Fixed an issue when evaluating an expression for a resource object when there
+  are no values at all for a property, but there is a list of associated data
+  (ids/extensions).
+
 ## [3.9.0] - 2023-11-09
 ### Added
 - support for user-defined functions.

demo/package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "fhirpath",
-  "version": "3.9.0",
+  "version": "3.9.1",
   "description": "A FHIRPath engine",
   "main": "src/fhirpath.js",
   "dependencies": {
@@ -55,5 +55,5 @@
     "fhirpath": "bin/fhirpath"
   },
   "repository": "github:HL7/fhirpath.js",
-  "license": "MIT"
+  "license": "SEE LICENSE in LICENSE.md"
 }

package-lock.json

@@ -5,9 +5,8 @@ const { deepEqual } = require('./deep-equal');
 const engine = {};
 
 
-// b is assumed to have one element and it tests whether b[0] is in a
+// "b" is assumed to have one element and it tests whether "b[0]" is in "a"
 function containsImpl(a,b){
-  if(b.length == 0) { return true; }
   for(var i = 0; i < a.length; i++){
     if(deepEqual(a[i], b[0])) { return true; }
   }

package.json

@@ -372,6 +372,18 @@ engine.MemberInvocation = function(ctx, parentData, node ) {
           if(Array.isArray(toAdd)) {
             acc = acc.concat(toAdd.map((x, i)=>
               makeResNode(x, childPath, _toAdd && _toAdd[i])));
+            // Add items to the end of the ResourceNode list that have no value
+            // but have associated data, such as extensions or ids.
+            const _toAddLength = _toAdd?.length || 0;
+            for (let i = toAdd.length; i < _toAddLength; ++i) {
+              acc.push(makeResNode(null, childPath, _toAdd[i]));
+            }
+          } else if (toAdd == null && Array.isArray(_toAdd)) {
+            // Add items to the end of the ResourceNode list when there are no
+            // values at all, but there is a list of associated data, such as
+            // extensions or ids.
+            acc = acc.concat(_toAdd.map((x) =>
+              makeResNode(null, childPath, x)));
           } else {
             acc.push(makeResNode(toAdd, childPath, _toAdd));
           }
@@ -680,7 +692,7 @@ function applyParsedPath(resource, parsedPath, context, model, options) {
     // Resolve any internal "ResourceNode" instances to plain objects and if
     // options.resolveInternalTypes is true, resolve any internal "FP_Type"
     // instances to strings.
-    .map(n => {
+    .reduce((acc,n) => {
       // Path for the data extracted from the resource.
       let path = n instanceof ResourceNode ? n.path : null;
       n = util.valData(n);
@@ -689,13 +701,17 @@ function applyParsedPath(resource, parsedPath, context, model, options) {
           n = n.toString();
         }
       }
-      // Add a hidden (non-enumerable) property with the path to the data extracted
-      // from the resource.
-      if (path && typeof n === 'object') {
-        Object.defineProperty(n, '__path__', {value: path});
+      // Exclude nulls
+      if (n != null) {
+        // Add a hidden (non-enumerable) property with the path to the data extracted
+        // from the resource.
+        if (path && typeof n === 'object') {
+          Object.defineProperty(n, '__path__', {value: path});
+        }
+        acc.push(n);
       }
-      return n;
-    });
+      return acc;
+    }, []);
 }
 
 /**

src/collections.js

@@ -24,8 +24,9 @@ function hashObject(obj) {
  */
 function prepareObject(value) {
   value = valDataConverted(value);
-
-  if (typeof value === 'number') {
+  if (value === null) {
+    return null;
+  } else if (typeof value === 'number') {
     return roundToMaxPrecision(value);
   } else if (value instanceof Date) {
     return value.toISOString();

src/fhirpath.js

@@ -44,7 +44,8 @@ child.on('exit', code => {
       'union',
       'exclude',
       'subsetof',
-      'distinct'
+      'distinct',
+      'contains'
     ].forEach(filename => {
       const suites = require('./benchmark/'+ filename)({
         benny,

src/hash-object.js

@@ -0,0 +1,50 @@
+const _ = require('lodash');
+
+module.exports = ({
+                    current_fhirpath,
+                    current_r4_model,
+                    bigItems,
+                    smallItems
+                  }) => {
+
+  const lastOfBigItemsCopy = _.cloneDeep(bigItems.slice(-1));
+  const numberOfBigItems = current_fhirpath.evaluate({}, '%items.count()', { items: bigItems }, current_r4_model)[0];
+
+  const lastOfSmallItemsCopy = _.cloneDeep(smallItems.slice(-1));
+  const numberOfSmallItems = current_fhirpath.evaluate({}, '%items.count()', { items: smallItems }, current_r4_model)[0];
+
+  const expression = '%items contains %lastOfItemsCopy';
+
+  return [{
+    name: `Checking if a list of items contains some item`,
+    filename: 'contains',
+    expression,
+    cases: [
+      {
+        name: `${numberOfBigItems} big items using evaluate()`,
+        testFunction: (fhirpath, model) => {
+          fhirpath.evaluate({}, expression, { items: bigItems, lastOfItemsCopy: lastOfBigItemsCopy }, model);
+        }
+      },
+      {
+        name: `${numberOfBigItems} big items using compile()`,
+        testFunction: (fhirpath, model, compiledFn) => {
+          compiledFn({}, { items: bigItems, lastOfItemsCopy: lastOfBigItemsCopy });
+        }
+      },
+      {
+        name: `${numberOfSmallItems} small items using evaluate()`,
+        testFunction: (fhirpath, model) => {
+          fhirpath.evaluate({}, expression, { items: smallItems, lastOfItemsCopy: lastOfSmallItemsCopy }, model);
+        }
+      },
+      {
+        name: `${numberOfSmallItems} small items using compile()`,
+        testFunction: (fhirpath, model, compiledFn) => {
+          compiledFn({}, { items: smallItems, lastOfItemsCopy: lastOfSmallItemsCopy });
+        }
+      }
+    ]
+  }];
+
+}

test/benchmark.js

@@ -354,6 +354,15 @@ tests:
       - prop1: 5
         prop2: 6
 
+  - desc: '** should ignore null properties'
+    inputfile: patient-bundle.json
+    model: r4
+    expression: >-
+      Bundle.entry.distinct() = (Bundle.entry[0] | Bundle.entry[2])
+    result:
+      - true
+
+
   - desc: '** should use year-to-month conversion factor (https://hl7.org/fhirpath/#equals)'
     expression: (1 year).combine(12 months).distinct()
     result: