From 85c32838ca19e40c8e9aa821a8ea453a9ff8826b Mon Sep 17 00:00:00 2001 From: sedinkinya Date: Wed, 2 Mar 2022 15:14:05 -0500 Subject: [PATCH 1/5] Support intersect() LF-2171 --- CHANGELOG.md | 4 ++++ package-lock.json | 2 +- package.json | 2 +- src/combining.js | 21 +++++++++++++++++++++ src/fhirpath.js | 1 + test/cases/fhir-r4.yaml | 1 - 6 files changed, 28 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7dc8bc..55b00bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ This log documents significant changes for each release. This project follows [Semantic Versioning](http://semver.org/). +## [2.14.0] - 2022-03-02 +### Added +- Function to get the intersection of two collections: intersect(). + ## [2.13.0] - 2022-02-28 ### Added - Current time function: timeOfDay(). diff --git a/package-lock.json b/package-lock.json index 4e4eb49..7de1ec2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "fhirpath", - "version": "2.13.0", + "version": "2.14.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 399bee9..6805e84 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fhirpath", - "version": "2.13.0", + "version": "2.14.0", "description": "A FHIRPath engine", "main": "src/fhirpath.js", "dependencies": { diff --git a/src/combining.js b/src/combining.js index b3958d2..146d62f 100644 --- a/src/combining.js +++ b/src/combining.js @@ -11,5 +11,26 @@ combineFns.combineFn = function(coll1, coll2){ return coll1.concat(coll2); }; +combineFns.intersect = function(coll1, coll2) { + let result = []; + if (coll1.length && coll2.length) { + let coll2json = {}; + coll2.forEach(item => { + coll2json[JSON.stringify(item)] = true; + }); + + for (let i=0, len=coll1.length; i Date: Thu, 3 Mar 2022 18:27:37 -0500 Subject: [PATCH 2/5] union(), distinct() and intersect() should not depend on the order of properties in an object LF-2171 --- src/combining.js | 9 +++++---- src/existence.js | 36 +++------------------------------ src/utilities.js | 37 ++++++++++++++++++++++++++++++++-- test/cases/5.1_existence.yaml | 19 +++++++++++++++++ test/cases/5.3_subsetting.yaml | 18 +++++++++++++++++ test/cases/5.4_combining.yaml | 20 ++++++++++++++++++ 6 files changed, 100 insertions(+), 39 deletions(-) diff --git a/src/combining.js b/src/combining.js index 146d62f..6d498bb 100644 --- a/src/combining.js +++ b/src/combining.js @@ -1,7 +1,8 @@ // This file holds code to hande the FHIRPath Combining functions. -var combineFns = {}; -var existence = require('./existence'); +const combineFns = {}; +const existence = require('./existence'); +const { orderedJsonStringify } = require("./utilities"); combineFns.union = function(coll1, coll2){ return existence.distinctFn(coll1.concat(coll2)); @@ -16,12 +17,12 @@ combineFns.intersect = function(coll1, coll2) { if (coll1.length && coll2.length) { let coll2json = {}; coll2.forEach(item => { - coll2json[JSON.stringify(item)] = true; + coll2json[orderedJsonStringify(item)] = true; }); for (let i=0, len=coll1.length; i { - const v = value[key]; - o[key] = sortObjByKey(v); - return o; - }, {}) - ) : - value; -} - - /** * Returns true if coll1 is a subset of coll2. */ @@ -110,13 +80,13 @@ function subsetOf(coll1, coll2) { var c2Hash = {}; for (let p=0, pLen=coll1.length; p { + const v = value[key]; + o[key] = sortObjByKey(v); + return o; + }, {}) + ) : + value; +} + module.exports = util; diff --git a/test/cases/5.1_existence.yaml b/test/cases/5.1_existence.yaml index 9825fa0..13d7cff 100644 --- a/test/cases/5.1_existence.yaml +++ b/test/cases/5.1_existence.yaml @@ -326,6 +326,16 @@ tests: expression: Functions.coll1[0].coll2.attr.distinct() result: [1, 2, 3] + - desc: '** should not depend on the order of properties in an object' + expression: Functions.objects.distinct() + result: + - prop1: 1 + prop2: 2 + - prop1: 3 + prop2: 4 + - prop1: 5 + prop2: 6 + - desc: '5.1.13. count() : integer' # Returns a collection with a single value which is @@ -410,4 +420,13 @@ subject: c: e: 6 d: 5 + objects: + - prop1: 1 + prop2: 2 + - prop1: 3 + prop2: 4 + - prop2: 2 + prop1: 1 + - prop1: 5 + prop2: 6 diff --git a/test/cases/5.3_subsetting.yaml b/test/cases/5.3_subsetting.yaml index 4f54d3c..2c20bb5 100644 --- a/test/cases/5.3_subsetting.yaml +++ b/test/cases/5.3_subsetting.yaml @@ -171,6 +171,13 @@ tests: expression: Functions.coll1.coll2.attr.take(5) result: [1, 2, 3, 4, 5] + - desc: '5.3.8. intersect(other: collection) : collection' + - desc: '** should not depend on the order of properties in an object' + expression: Functions.objects.group1.intersect(Functions.objects.group2) + result: + - prop1: 1 + prop2: 2 + subject: resourceType: Functions attrempty: [] @@ -216,3 +223,14 @@ subject: - attr: '@2015-02-04T14:34:28Z' - attr: '@T14:34:28+09:00' - attr: 4 days + objects: + group1: + - prop1: 1 + prop2: 2 + - prop1: 3 + prop2: 4 + group2: + - prop2: 2 + prop1: 1 + - prop1: 5 + prop2: 6 diff --git a/test/cases/5.4_combining.yaml b/test/cases/5.4_combining.yaml index 34598ea..78aa85c 100644 --- a/test/cases/5.4_combining.yaml +++ b/test/cases/5.4_combining.yaml @@ -20,6 +20,15 @@ tests: expression: Functions.attrdouble | Functions.coll1.coll2.attr result: [1, 2, 3, 4, 5] + - desc: '** should not depend on the order of properties in an object' + expression: Functions.objects.group1 | Functions.objects.group2 + result: + - prop1: 1 + prop2: 2 + - prop1: 3 + prop2: 4 + - prop1: 5 + prop2: 6 - desc: '5.4.2. combine(other : collection) : collection' # Merge the input and other collections into a single collection without eliminating duplicate values. Combining an empty collection with a non-empty collection will return the non-empty collection. There is no expectation of order in the resulting collection. @@ -85,3 +94,14 @@ subject: - attr: '@2015-02-04T14:34:28Z' - attr: '@T14:34:28+09:00' - attr: 4 days + objects: + group1: + - prop1: 1 + prop2: 2 + - prop1: 3 + prop2: 4 + group2: + - prop2: 2 + prop1: 1 + - prop1: 5 + prop2: 6 From aa00813a1dc38d027deb7ddd35b24bac6762237e Mon Sep 17 00:00:00 2001 From: sedinkinya Date: Mon, 14 Mar 2022 15:53:26 -0400 Subject: [PATCH 3/5] Fixed issues found during review LF-2171 --- src/combining.js | 18 ++++-------------- src/deep-equal.js | 6 +----- src/existence.js | 38 ++++++++------------------------------ src/utilities.js | 33 --------------------------------- test/cases/3.2_paths.yaml | 2 +- 5 files changed, 14 insertions(+), 83 deletions(-) diff --git a/src/combining.js b/src/combining.js index 6d498bb..70a4d80 100644 --- a/src/combining.js +++ b/src/combining.js @@ -2,7 +2,7 @@ const combineFns = {}; const existence = require('./existence'); -const { orderedJsonStringify } = require("./utilities"); +const deepEqual = require('./deep-equal'); combineFns.union = function(coll1, coll2){ return existence.distinctFn(coll1.concat(coll2)); @@ -15,19 +15,9 @@ combineFns.combineFn = function(coll1, coll2){ combineFns.intersect = function(coll1, coll2) { let result = []; if (coll1.length && coll2.length) { - let coll2json = {}; - coll2.forEach(item => { - coll2json[orderedJsonStringify(item)] = true; - }); - - for (let i=0, len=coll1.length; i coll2.some(obj2 => deepEqual(obj1, obj2)) + ) } return result; diff --git a/src/deep-equal.js b/src/deep-equal.js index be3713b..3f3c30c 100644 --- a/src/deep-equal.js +++ b/src/deep-equal.js @@ -54,12 +54,8 @@ var deepEqual = function (actual, expected, opts) { if (actual instanceof Date && expected instanceof Date) { return actual.getTime() === expected.getTime(); - - // 7.3. Other pairs that do not both pass typeof value == 'object', - // equivalence is determined by ==. } else if (!actual || !expected || typeof actual != 'object' && typeof expected != 'object') { - return opts.strict ? actual === expected : actual == expected; - + return actual === expected; } else { var actualIsFPT = actual instanceof FP_Type; diff --git a/src/existence.js b/src/existence.js index f3d38e0..fea9c56 100644 --- a/src/existence.js +++ b/src/existence.js @@ -4,6 +4,7 @@ const util = require("./utilities"); const filtering = require("./filtering"); const misc = require("./misc"); +const deepEqual = require('./deep-equal'); const engine = {}; engine.emptyFn = util.isEmpty; @@ -74,26 +75,9 @@ engine.anyFalseFn = function(x) { function subsetOf(coll1, coll2) { let rtn = coll1.length <= coll2.length; if (rtn) { - // This requires a deep-equals comparision of every object in coll1, - // against each object in coll2. - // Optimize by building a hashmap of JSON versions of the objects. - var c2Hash = {}; for (let p=0, pLen=coll1.length; p deepEqual(obj1, util.valData(obj2))); } } return rtn; @@ -113,19 +97,13 @@ engine.isDistinctFn = function(x) { engine.distinctFn = function(x) { let unique = []; - // Since this requires a deep equals, use a hash table (on JSON strings) for - // efficiency. if (x.length > 0) { - let uniqueHash = {}; - for (let i=0, len=x.length; i !deepEqual(xObj, o, {strict: true})); + } while (x.length) } return unique; }; diff --git a/src/utilities.js b/src/utilities.js index b476e7e..27d7ece 100644 --- a/src/utilities.js +++ b/src/utilities.js @@ -101,37 +101,4 @@ util.escapeStringForRegExp = function (str) { return str.replace(/[-[\]{}()*+?.,\\/^$|#\s]/g, '\\$&'); }; -/** - * Returns a JSON version of the given object, but with keys of the object in - * sorted order (or at least a stable order). - * From: https://stackoverflow.com/a/35810961/360782 - */ -util.orderedJsonStringify = function(obj) { - return JSON.stringify(sortObjByKey(obj)); -}; - -/** - * If given value is an object, returns a new object with the properties added - * in sorted order, and handles nested objects. Otherwise, returns the given - * value. - * From: https://stackoverflow.com/a/35810961/360782 - */ -function sortObjByKey(value) { - // A special case for ResourceNode, which has its own toJSON method - if (value instanceof ResourceNode) { - return sortObjByKey(value.data); - } - return (typeof value === 'object') ? - (Array.isArray(value) ? - value.map(sortObjByKey) : - Object.keys(value).sort().reduce( - (o, key) => { - const v = value[key]; - o[key] = sortObjByKey(v); - return o; - }, {}) - ) : - value; -} - module.exports = util; diff --git a/test/cases/3.2_paths.yaml b/test/cases/3.2_paths.yaml index 3095d44..205a492 100644 --- a/test/cases/3.2_paths.yaml +++ b/test/cases/3.2_paths.yaml @@ -50,7 +50,7 @@ tests: result: ['medium', 'low', 'zero'] - desc: "QR with where()" - expression: "contained.where(resourceType = 'QuestionnaireResponse').item.where(linkId = 1).answer.value" + expression: "contained.where(resourceType = 'QuestionnaireResponse').item.where(linkId = '1').answer.value" model: 'r4' result: ['Red'] From 024fac74a5363eadd824df7444e22d4a6f3cae59 Mon Sep 17 00:00:00 2001 From: sedinkinya Date: Wed, 16 Mar 2022 15:47:15 -0400 Subject: [PATCH 4/5] Minor fixes LF-2171 --- src/combining.js | 2 +- src/existence.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/combining.js b/src/combining.js index 70a4d80..52f80ca 100644 --- a/src/combining.js +++ b/src/combining.js @@ -17,7 +17,7 @@ combineFns.intersect = function(coll1, coll2) { if (coll1.length && coll2.length) { result = existence.distinctFn(coll1).filter( obj1 => coll2.some(obj2 => deepEqual(obj1, obj2)) - ) + ); } return result; diff --git a/src/existence.js b/src/existence.js index fea9c56..ee1d95f 100644 --- a/src/existence.js +++ b/src/existence.js @@ -102,8 +102,8 @@ engine.distinctFn = function(x) { do { let xObj = x.shift(); unique.push(xObj); - x = x.filter(o => !deepEqual(xObj, o, {strict: true})); - } while (x.length) + x = x.filter(o => !deepEqual(xObj, o)); + } while (x.length); } return unique; }; From 1a24c742108781275e6721696ce5738713ffa60d Mon Sep 17 00:00:00 2001 From: sedinkinya Date: Fri, 18 Mar 2022 14:04:29 -0400 Subject: [PATCH 5/5] Updated CHANGELOG.md LF-2171 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55b00bf..6a19d09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ This log documents significant changes for each release. This project follows ## [2.14.0] - 2022-03-02 ### Added - Function to get the intersection of two collections: intersect(). +### Fixed +- The distinct, union, subsetOf, and intersect functions now use + the "6.1.1. = (Equals)" function to compare collection items instead of using + a map with JSON keys, which can affect their performance because the + complexity of the algorithm has changed from O(n) to O(n**2). ## [2.13.0] - 2022-02-28 ### Added