Merge branch 'bugfix/LF-3330/use-GET-for-memberOf-when-possible' into 'master'

yuriy-sedinkin · yuriy-sedinkin · commit a198b963573b · 2025-07-15T21:04:27.000Z
Use GET for the memberOf() function whenever possible

See merge request lfor/fhirpath.js!43
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,9 +3,13 @@
 This log documents significant changes for each release.  This project follows
 [Semantic Versioning](http://semver.org/).
 
-## [4.5.1] - 2025-07-07
+## [4.5.1] - 2025-07-09
 ### Added
 - tests for the R5 model.
+### Changed
+- Updated the "%terminologies.validateVS()" method and "memberOf()" function to
+  use GET for CodeableConcept with a single Coding when using a ValueSet
+  reference instead of an actual ValueSet.
 ### Fixed
 - updated tests for the R4 model.
 
diff --git a/src/additional.js b/src/additional.js
@@ -36,7 +36,7 @@ engine.memberOf = function (coll, valueSetColl ) {
       }
       return Terminologies.validateVS.call(this,
         [terminologies], valueSetColl, coll, ''
-      ).then(params => {
+      )?.then(params => {
         return util.valData(params)?.parameter.find((p) => p.name === "result").valueBoolean;
       }, () => []);
     }
diff --git a/src/terminologies.js b/src/terminologies.js
@@ -172,23 +172,28 @@ class Terminologies {
     const ctx = this;
     util.checkAllowAsync(ctx, 'validateVS');
 
-    if(valueSetColl.length === 1 && codedColl.length === 1 &&
-      checkParams(params)) {
+    const valueSet = valueSetColl.length === 1 && util.valData(valueSetColl[0]);
+    let coded = codedColl.length === 1 && util.valData(codedColl[0]);
+
+    // If valueSet or coded are empty, we can predict that the $validate-code
+    // operation will return an error.
+    if(valueSet && coded && checkParams(params)) {
       const vsTypeInfo = TypeInfo.fromValue(valueSetColl[0]);
       const isActualValueSet = vsTypeInfo.is(TypeInfo.FhirValueSet, ctx.model);
       const isValueSetUrl = vsTypeInfo.is(TypeInfo.FhirUri, ctx.model) ||
         vsTypeInfo.is(TypeInfo.SystemString, ctx.model);
       if (isActualValueSet || isValueSetUrl) {
         const {isCodeableConcept, isCoding, isCode} = getCodedType(ctx, codedColl);
         if (isCodeableConcept || isCoding || isCode) {
-          const valueSet = util.valData(valueSetColl[0]);
-          const coded = util.valData(codedColl[0]);
           const requestUrl =
             `${self[0].terminologyUrl}/ValueSet/$validate-code`;
 
-          if (isActualValueSet || isCodeableConcept) {
-            // Workaround for the case where we don't have a system.
-            // See discussion here:
+          // Use a POST request if the passed valueSet is an actual ValueSet or
+          // the passed coded value is a CodeableConcept with more than one
+          // coding or no coding.
+          if (isActualValueSet || isCodeableConcept && coded.coding?.length !== 1) {
+            // getSystemFromVS() is a workaround for the case where we don't
+            // have a system. See discussion here:
             //  https://chat.fhir.org/#narrow/stream/179266-fhirpath/topic/Problem.20with.20the.20.22memberOf.22.20function.20and.20R4.20servers
             response = (isCode ?
               getSystemFromVS(ctx, self[0].terminologyUrl, valueSet)
@@ -218,7 +223,7 @@ class Terminologies {
                 }
               );
             });
-          } else  {
+          } else  { // Otherwise use a GET request.
             if (isCode) {
               // Workaround for the case where we don't have a system.
               // See discussion here:
@@ -237,18 +242,27 @@ class Terminologies {
                     }
                   );
                 });
-            } else if (isCoding) {
-              const queryParams = new URLSearchParams({
-                url: valueSet ?? '',
-                system: coded.system ?? '',
-                code: coded.code
-              });
-              response = util.fetchWithCache(
-                `${requestUrl}?${queryParams.toString() + (params ? '&' + params : '')}`,
-                {
-                  ...(ctx.signal ? {signal: ctx.signal} : {})
-                }
-              );
+            } else {
+              // If the coded value is a CodeableConcept with only one Coding
+              if (isCodeableConcept) {
+                coded = coded.coding[0];
+              }
+              // If the coded value is Coding and has system and code, we can
+              // use it in the request URL; otherwise, the $validate-code
+              // operation will return an error.
+              if (coded?.system && coded?.code) {
+                const queryParams = new URLSearchParams({
+                  url: valueSet,
+                  system: coded.system,
+                  code: coded.code
+                });
+                response = util.fetchWithCache(
+                  `${requestUrl}?${queryParams.toString() + (params ? '&' + params : '')}`,
+                  {
+                    ...(ctx.signal ? {signal: ctx.signal} : {})
+                  }
+                );
+              }
             }
           }
         }
@@ -779,19 +793,19 @@ function getCodedType(ctx, codedColl) {
  *  object or null.
  * @param {string} resourceType - The expected FHIR resource type (e.g.,
  *  "ValueSet", "Parameters").
- * @returns {Promise<ResourceNode|null>} - A promise resolving to a ResourceNode
- *  if the resource type matches, or null if an error occurs or the resource
- *  type does not match.
+ * @returns {Promise<ResourceNode|null>|null} - A promise resolving to a ResourceNode
+ *  if the resource type matches, or to null if an error occurs or the resource
+ *  type does not match; or null if the given response object is falsy.
  */
 function transformResponseToResource(ctx, response, resourceType) {
-  return response && response.then(obj => {
+  return response?.then(obj => {
     if (obj?.resourceType === resourceType) {
       return ResourceNode.makeResNode(obj, null, null, null, null, ctx.model);
     }
     // Throw an error if the resource type does not match - will cause the catch
     // function to be called.
     throw new Error('Unexpected resourceType in response: ' + obj?.resourceType);
-  }).catch(() => null);
+  }).catch(() => null) || null;
 }
 
 module.exports = Terminologies;
diff --git a/test/async-functions.test.js b/test/async-functions.test.js
@@ -412,6 +412,52 @@ describe('Async functions', () => {
       expect(result).toEqual([]);
     });
 
+    it('should return an empty collection when a code does not have a value', () => {
+      mockFetchResults([
+        ['ValueSet?url=http%3A%2F%2Fhl7.org%2Ffhir%2FValueSet%2Fobservation-vitalsignresult', {
+          "resourceType": "Bundle",
+          "entry": [{
+            "resource": {
+              "resourceType": "ValueSet",
+              "url": "http://hl7.org/fhir/ValueSet/observation-vitalsignresult",
+              "compose": {
+                "include": [{
+                  "system": "http://loinc.org",
+                  "concept": [
+                    {"code": "85353-1"}, {"code": "9279-1"}, {"code": "8867-4"},
+                    {"code": "2708-6"}, {"code": "8310-5"}, {"code": "8302-2"},
+                    {"code": "9843-4"}, {"code": "29463-7"},
+                    {"code": "39156-5"}, {"code": "85354-9"},
+                    {"code": "8480-6"}, {"code": "8462-4"}, {"code": "8478-0"}
+                  ]
+                }]
+              }
+            }
+          }]
+        }]
+      ]);
+
+      let result = fhirpath.evaluate(
+        {
+          "resourceType": "Observation",
+          "code": {
+            "coding":
+              [{
+                "_code": {
+                  "id": "someCodeId"
+                }
+              }]
+          }
+        },
+        "%terminologies.validateVS('http://hl7.org/fhir/ValueSet/observation-vitalsignresult', Observation.code.coding.code)",
+        {},
+        modelR4,
+        { async: true, terminologyUrl: "https://lforms-fhir.nlm.nih.gov/baseR4" }
+      );
+
+      expect(result).toEqual([]);
+    });
+
 
     it('should throw an error when the async function is not allowed', () => {
       let result = () => fhirpath.evaluate(
@@ -682,7 +728,11 @@ describe('Async functions', () => {
 
     it('should work with CodeableConcept when async functions are enabled', (done) => {
       mockFetchResults([
-        [/ValueSet\/\$validate-code/, {
+        [{
+          url: '/ValueSet/$validate-code',
+          body: '"system":"http://loinc.org"',
+          method: 'POST'
+        }, {
           "resourceType": "Parameters",
           "parameter": [
             {
@@ -707,6 +757,79 @@ describe('Async functions', () => {
     });
 
 
+    it('should use GET for CodeableConcept with a single Coding', (done) => {
+      mockFetchResults([
+        [{
+          url: /ValueSet\/\$validate-code.*&system=system1/,
+          method: 'GET'
+        }, {
+          "resourceType": "Parameters",
+          "parameter": [
+            {
+              "name": "result",
+              "valueBoolean": true
+            }
+          ]
+        }]
+      ]);
+      let result = fhirpath.evaluate(
+        observationResource,
+        "%factory.CodeableConcept(%factory.Coding('system1', '1')).memberOf('http://some-valueset')",
+        {},
+        modelR4,
+        { async: true, terminologyUrl: "https://lforms-fhir.nlm.nih.gov/baseR4" }
+      );
+      expect(result instanceof Promise).toBe(true);
+      result.then((r) => {
+        expect(r).toEqual([true]);
+        done();
+      })
+    });
+
+    it('should work when Coding in a CodeableConcept has no system and code', () => {
+      mockFetchResults([]);
+      let result = fhirpath.evaluate(
+        {},
+        "%factory.CodeableConcept(%factory.Coding('', '', 'some display text')).memberOf('http://some-valueset')",
+        {},
+        modelR4,
+        { async: true, terminologyUrl: "https://lforms-fhir.nlm.nih.gov/baseR4" }
+      );
+      expect(result).toEqual([]);
+    });
+
+
+    it('should work when no Coding in a CodeableConcept', (done) => {
+      mockFetchResults([
+        [{
+          url: '/ValueSet/$validate-code',
+          body: '"valueUri":"http://some-valueset"',
+          method: 'POST'
+        }, null, {
+          "resourceType": "OperationOutcome",
+          "issue": [ {
+            "severity": "error",
+            "code": "processing",
+            "diagnostics": "HAPI-0899: No code, coding, or codeableConcept provided to validate"
+          } ]
+        }]
+      ]);
+      let result = fhirpath.evaluate(
+        {},
+        "%factory.CodeableConcept(Observation.code.coding).memberOf('http://some-valueset')",
+        {},
+        modelR4,
+        { async: true, terminologyUrl: "https://lforms-fhir.nlm.nih.gov/baseR4" }
+      );
+      expect(result instanceof Promise).toBe(true);
+      result.then((r) => {
+        expect(r).toEqual([]);
+        done();
+      });
+    });
+
+
+
     it('should work with "code" when async functions are enabled', (done) => {
       mockFetchResults([
         [/ValueSet\?url=http%3A%2F%2Fhl7\.org%2Ffhir%2FValueSet%2Fobservation-vitalsignresult/, {
diff --git a/test/mock-fetch-results.js b/test/mock-fetch-results.js
@@ -3,20 +3,26 @@ let fetchSpy;
 
 /**
  * Checks whether a string (in the first parameter) matches a regular expression,
- * substring or test function (in the second parameter).
+ * contains the substring or satisfies the test function (in the second parameter).
  * If the second parameter is not passed (no condition), returns true.
  * @param {string} str - string to check
  * @param {RegExp|string|Function|undefined} condition - regular expression,
  *  substring, or test function.
+ * @param {Object} [options] - apply condition options:
+ * @param {Object} [options.fullStringMatch] - whether to perform a full string
+ *  comparison on the strings in the "str" and "condition" parameters, defaults
+ *  to false.
  * @returns {boolean}
  */
-function checkString(str, condition) {
+function checkString(str, condition, options= {}) {
   if (condition === undefined) {
     return true;
   } else if (condition instanceof RegExp) {
     return condition.test(str);
   } else if (typeof condition === 'string') {
-    return str && (str.indexOf(condition) !== -1)
+    return options?.fullStringMatch ?
+      str === condition
+      : (str !== null && str !== undefined && str.indexOf(condition) !== -1);
   } else if (condition instanceof Function) {
     return condition(str);
   } else {
@@ -36,19 +42,26 @@ function checkString(str, condition) {
  * response as the third item.
  * For mocking POST requests the first item of a response description with a URL
  * condition could be replaced with an object:
- * {url: string|Regexp, body: string|RegExp},
+ * {url: string|Regexp, body: string|RegExp, method: string},
  * where "url" is a RegExp for a URL or a URL substring,
- * "body" is a RegExp for the body content or a substring of the body content.
+ * "body" is a RegExp for the body content or a substring of the body content,
+ * and "method" is a RegExp for the HTTP method (e.g. "POST") or a substring of
+ * the HTTP method.
+ *
  * @param {Object} options - options for the mock.
  * @param {number} [options.timeout=0] - timeout for the mock response.
  */
 function mockFetchResults(results, {timeout = 0} = {}) {
   fetchSpy = jest.spyOn(global, 'fetch').mockImplementation(
-    (url, options) => new Promise((resolve, reject) => {
+    (url, options) => new Promise((resolve) => {
       const mockedItem = results?.find(
         (r) => {
-          if (typeof r[0] === 'object' && (r[0].url || r[0].body)) {
-            return checkString(url, r[0]?.url) && checkString(options.body, r[0]?.body);
+          if (typeof r[0] === 'object' && r[0] !== null && (r[0].url ||
+            r[0].body || r[0].method)) {
+            return checkString(url, r[0]?.url) &&
+              checkString(options.body, r[0]?.body) &&
+              checkString(options.method ?? 'GET', r[0]?.method,
+                {fullStringMatch: true});
           } else {
             return checkString(url, r[0]);
           }
@@ -60,7 +73,7 @@ function mockFetchResults(results, {timeout = 0} = {}) {
         options?.method === 'POST' &&
         optionHeaders.get('Content-Type') !== 'application/fhir+json; charset=utf-8'
       ) {
-        reportError('Unexpected request header.', reject);
+        reportError('Unexpected request header.');
       }
 
       const okResult = mockedItem?.[1];
@@ -84,7 +97,7 @@ function mockFetchResults(results, {timeout = 0} = {}) {
             ok: false
           });
         } else {
-          reportError(`"${url}" is not mocked.`, reject);
+          reportError(`"${url}" is not mocked.`);
         }
       }, timeout);
     })
@@ -94,13 +107,12 @@ function mockFetchResults(results, {timeout = 0} = {}) {
 /**
  * Report an error message.
  * @param {string} msg - error message to report.
- * @param {Function} rejectFn - function to call to reject the promise.
  */
-function reportError(msg, rejectFn) {
+function reportError(msg) {
   // Show error message in the console:
   console.error(msg);
-  // Reject the promise so that the request to the server fails:
-  rejectFn(msg);
+  // Throw an error so that the test fails:
+  throw new Error(msg);
 }
 
 

Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ engine.memberOf = function (coll, valueSetColl ) {`
`36`	`36`	`}`
`37`	`37`	`return Terminologies.validateVS.call(this,`
`38`	`38`	`[terminologies], valueSetColl, coll, ''`
`39`		`- ).then(params => {`
	`39`	`+ )?.then(params => {`
`40`	`40`	`return util.valData(params)?.parameter.find((p) => p.name === "result").valueBoolean;`
`41`	`41`	`}, () => []);`
`42`	`42`	`}`