From 8bccace0f8b39b384b09861e0148c3216e040b4b Mon Sep 17 00:00:00 2001 From: Dylan Phelan Date: Mon, 12 Apr 2021 18:13:11 -0400 Subject: [PATCH 1/3] Added fn to find all invalid resources in a bundle --- src/client/BaseClient.js | 4 +-- src/helpers/fhirUtils.js | 19 ++++++++++++-- test/helpers/fhirUtils.test.js | 28 +++++++++++++++++++++ test/helpers/fixtures/invalid-resource.json | 16 ++++++++++++ test/helpers/fixtures/valid-resource.json | 16 ++++++++++++ 5 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 test/helpers/fixtures/invalid-resource.json create mode 100644 test/helpers/fixtures/valid-resource.json diff --git a/src/client/BaseClient.js b/src/client/BaseClient.js index da848882..755c6ac9 100644 --- a/src/client/BaseClient.js +++ b/src/client/BaseClient.js @@ -1,4 +1,4 @@ -const { isValidFHIR } = require('../helpers/fhirUtils'); +const { isValidFHIR, invalidResourcesFromBundle } = require('../helpers/fhirUtils'); const logger = require('../helpers/logger'); class BaseClient { @@ -97,7 +97,7 @@ class BaseClient { }, Promise.resolve(contextBundle)); if (!isValidFHIR(contextBundle)) { - logger.error('Extracted bundle is not valid FHIR.'); + logger.error(`Extracted bundle is not valid FHIR, the following resources failed validation: ${invalidResourcesFromBundle(contextBundle).join(',')}`); } return { bundle: contextBundle, extractionErrors }; diff --git a/src/helpers/fhirUtils.js b/src/helpers/fhirUtils.js index 81d1b3c8..e98969cc 100644 --- a/src/helpers/fhirUtils.js +++ b/src/helpers/fhirUtils.js @@ -163,8 +163,22 @@ const logOperationOutcomeInfo = (operationOutcome) => { }); }; -const isValidFHIR = (resource) => validator.validate('FHIR', resource); - +function isValidFHIR(resource) { + return validator.validate('FHIR', resource); +} +function invalidResourcesFromBundle(bundle) { + // Bundle is assumed to have all resources in bundle.entry[x].resource + const failingResources = []; + bundle.entry.forEach((e) => { + const { resource } = e; + const { id, resourceType } = resource; + if (!validator.validate('FHIR', resource)) { + const failureId = `${resourceType}-${id}`; + failingResources.push(failureId); + } + }); + return failingResources; +} module.exports = { allResourcesInBundle, @@ -182,4 +196,5 @@ module.exports = { mapFHIRVersions, quantityCodeToUnitLookup, isValidFHIR, + invalidResourcesFromBundle, }; diff --git a/test/helpers/fhirUtils.test.js b/test/helpers/fhirUtils.test.js index da841915..e2213f43 100644 --- a/test/helpers/fhirUtils.test.js +++ b/test/helpers/fhirUtils.test.js @@ -7,6 +7,8 @@ const { allResourcesInBundle, quantityCodeToUnitLookup, getResourceCountInBundle, + isValidFHIR, + invalidResourcesFromBundle, } = require('../../src/helpers/fhirUtils.js'); const emptyBundle = require('./fixtures/emptyBundle.json'); const bundleWithOneEntry = require('./fixtures/searchsetBundleWithOneEntry.json'); @@ -14,6 +16,8 @@ const bundleWithMultipleEntries = require('./fixtures/searchsetBundleWithMultipl const countBundle5Unique = require('./fixtures/count-bundle-5-unique.json'); const countBundle5Same = require('./fixtures/count-bundle-5-same.json'); const countBundle5Nested = require('./fixtures/count-bundle-5-nested.json'); +const validResource = require('./fixtures/valid-resource'); +const invalidResource = require('./fixtures/invalid-resource'); describe('getQuantityUnit', () => { test('Should return unit text if provided in lookup table', () => { @@ -137,3 +141,27 @@ describe('getResourceCountInBundle', () => { expect(getResourceCountInBundle(countBundle5Nested)).toEqual(counts); }); }); + +describe('isValidFHIR', () => { + test('Should return true when provided valid FHIR resources', () => { + expect(isValidFHIR(bundleWithOneEntry)).toEqual(true); + }); + test('Should return false when provided invalid FHIR resources', () => { + const invalidBundle = { ...bundleWithOneEntry }; + invalidBundle.entry[0].resource = invalidResource; + expect(isValidFHIR(invalidBundle)).toEqual(false); + }); +}); + +describe('invalidResourcesFromBundle', () => { + test('Should return an empty array when all resources are valid', () => { + expect(invalidResourcesFromBundle(emptyBundle)).toEqual([]); + }); + test('Should return an array of all invalid resources when they exist', () => { + const invalidBundle = { ...bundleWithOneEntry }; + invalidBundle.entry[0].resource = invalidResource; + // This is dependent on implementation details intrinsic to invalidResourcesFromBundle + const invalidResourceId = `${invalidResource.resourceType}-${invalidResource.id}`; + expect(invalidResourcesFromBundle(invalidBundle)).toEqual([invalidResourceId]); + }); +}); diff --git a/test/helpers/fixtures/invalid-resource.json b/test/helpers/fixtures/invalid-resource.json new file mode 100644 index 00000000..f56e4e93 --- /dev/null +++ b/test/helpers/fixtures/invalid-resource.json @@ -0,0 +1,16 @@ +{ + "resourceType": "Patient", + "id": "invalid-patient", + "name": [ + { + "family": [ + "Godel" + ], + "given": [ + "Kurt" + ] + } + ], + "gender": "invalid-enum-value", + "birthDate": "1906-04-28" +} diff --git a/test/helpers/fixtures/valid-resource.json b/test/helpers/fixtures/valid-resource.json new file mode 100644 index 00000000..10d6a70a --- /dev/null +++ b/test/helpers/fixtures/valid-resource.json @@ -0,0 +1,16 @@ +{ + "resourceType": "Patient", + "id": "valid-patient", + "name": [ + { + "family": [ + "Frege" + ], + "given": [ + "Gottlob" + ] + } + ], + "gender": "male", + "birthDate": "1848-11-08" +} From 65a7fd0c7212d59af3ee13de590b040d5a5d356b Mon Sep 17 00:00:00 2001 From: Dylan Phelan Date: Mon, 12 Apr 2021 18:20:57 -0400 Subject: [PATCH 2/3] error message is now a warning --- src/client/BaseClient.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/BaseClient.js b/src/client/BaseClient.js index 755c6ac9..feaa6940 100644 --- a/src/client/BaseClient.js +++ b/src/client/BaseClient.js @@ -97,7 +97,7 @@ class BaseClient { }, Promise.resolve(contextBundle)); if (!isValidFHIR(contextBundle)) { - logger.error(`Extracted bundle is not valid FHIR, the following resources failed validation: ${invalidResourcesFromBundle(contextBundle).join(',')}`); + logger.warn(`Extracted bundle is not valid FHIR, the following resources failed validation: ${invalidResourcesFromBundle(contextBundle).join(',')}`); } return { bundle: contextBundle, extractionErrors }; From b3aaff8e9a3fc398d7a3a073c7a8380aad46b2b8 Mon Sep 17 00:00:00 2001 From: Dylan Phelan Date: Mon, 12 Apr 2021 18:31:33 -0400 Subject: [PATCH 3/3] fixing lint and cross-test errs --- test/client/BaseClient.test.js | 4 ++-- test/helpers/fhirUtils.test.js | 6 ++---- test/helpers/fixtures/invalid-resource.json | 4 +--- test/helpers/fixtures/valid-resource.json | 4 +--- 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/test/client/BaseClient.test.js b/test/client/BaseClient.test.js index 7ca874fb..a77de7f9 100644 --- a/test/client/BaseClient.test.js +++ b/test/client/BaseClient.test.js @@ -76,8 +76,8 @@ describe('BaseClient', () => { }); it('should iterate over all extractors gets, aggregating resultant entries in a bundle', async () => { const extractorClassesWithEntryGets = [ - class Ext1 { get() { return { entry: [{ data: 'here' }] }; }}, - class Ext2 { get() { return { entry: [{ data: 'alsoHere' }] }; }}, + class Ext1 { get() { return { entry: [{ resource: 'here' }] }; }}, + class Ext2 { get() { return { entry: [{ resource: 'alsoHere' }] }; }}, class Ext3 { get() { throw new Error('Error'); }}, ]; engine.registerExtractors(...extractorClassesWithEntryGets); diff --git a/test/helpers/fhirUtils.test.js b/test/helpers/fhirUtils.test.js index e2213f43..7b8b9425 100644 --- a/test/helpers/fhirUtils.test.js +++ b/test/helpers/fhirUtils.test.js @@ -144,12 +144,10 @@ describe('getResourceCountInBundle', () => { describe('isValidFHIR', () => { test('Should return true when provided valid FHIR resources', () => { - expect(isValidFHIR(bundleWithOneEntry)).toEqual(true); + expect(isValidFHIR(validResource)).toEqual(true); }); test('Should return false when provided invalid FHIR resources', () => { - const invalidBundle = { ...bundleWithOneEntry }; - invalidBundle.entry[0].resource = invalidResource; - expect(isValidFHIR(invalidBundle)).toEqual(false); + expect(isValidFHIR(invalidResource)).toEqual(false); }); }); diff --git a/test/helpers/fixtures/invalid-resource.json b/test/helpers/fixtures/invalid-resource.json index f56e4e93..f4fc75bc 100644 --- a/test/helpers/fixtures/invalid-resource.json +++ b/test/helpers/fixtures/invalid-resource.json @@ -3,9 +3,7 @@ "id": "invalid-patient", "name": [ { - "family": [ - "Godel" - ], + "family": "Godel", "given": [ "Kurt" ] diff --git a/test/helpers/fixtures/valid-resource.json b/test/helpers/fixtures/valid-resource.json index 10d6a70a..a9050caf 100644 --- a/test/helpers/fixtures/valid-resource.json +++ b/test/helpers/fixtures/valid-resource.json @@ -3,9 +3,7 @@ "id": "valid-patient", "name": [ { - "family": [ - "Frege" - ], + "family": "Frege", "given": [ "Gottlob" ]