From d678c0d81419050cdc4f26bb8b116fcf45e60665 Mon Sep 17 00:00:00 2001 From: kjefferson Date: Wed, 7 Aug 2024 14:40:23 -0400 Subject: [PATCH 1/7] adding initial check for self referencing value sets and logs error --- src/export/ValueSetExporter.ts | 19 +++++++++++++ test/export/ValueSetExporter.test.ts | 41 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts index e27ca9ed7..bcddcf50d 100644 --- a/src/export/ValueSetExporter.ts +++ b/src/export/ValueSetExporter.ts @@ -448,6 +448,25 @@ export class ValueSetExporter { ); } + // check value sets to ensure not self referencing + let selfReferencedIncludeValueSets = vs?.compose?.include?.[0]?.valueSet?.filter((valueSetItem) => valueSetItem == vs.url) ?? []; + let selfReferencedExcludeValueSets = vs?.compose?.exclude?.[0]?.valueSet?.filter((valueSetItem) => valueSetItem == vs.url) ?? []; + + if ( selfReferencedIncludeValueSets.length > 0 || selfReferencedExcludeValueSets.length > 0 ) { + logger.error( + `Value set with id ${vs.id} has component rule with self referencing value sets (by id, value set name, or url). Skipping rule.` + ) + // remove the self referenced rules for the output + // if (selfReferencedIncludeValueSets.length > 0 ) { + // let nonSelfReferenceValueSets = vs.compose.include[0].valueSet.filter((valueSetItem) => valueSetItem != vs.url) + // vs.compose.include[0].valueSet = nonSelfReferenceValueSets; + // } + // if (selfReferencedExcludeValueSets.length > 0 ) { + // let nonSelfReferenceValueSets = vs.compose.exclude[0].valueSet.filter((valueSetItem) => valueSetItem != vs.url) + // vs.compose.exclude[0].valueSet = nonSelfReferenceValueSets; + // } + } + cleanResource(vs, (prop: string) => ['_sliceName', '_primitive'].includes(prop)); this.pkg.valueSets.push(vs); this.pkg.fshMap.set(vs.getFileName(), { diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts index b6f9a35a4..5161c3140 100644 --- a/test/export/ValueSetExporter.test.ts +++ b/test/export/ValueSetExporter.test.ts @@ -473,6 +473,47 @@ describe('ValueSetExporter', () => { }); }); + it('should log error when exporting a value set that includes a component from a self referencing value set', () => { + const valueSet = new FshValueSet('DinnerVS'); + valueSet.id = "dinner-vs" + const component = new ValueSetConceptComponentRule(true); + component.from = { + valueSets: [ + 'http://food.org/food/ValueSet/hot-food', + 'http://food.org/food/ValueSet/cold-food', + 'DinnerVS', + 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', + 'dinner-vs' + ] + }; + valueSet.rules.push(component); + doc.valueSets.set(valueSet.name, valueSet); + const exported = exporter.export().valueSets; + expect(exported.length).toBe(1); + expect(exported[0]).toEqual({ + resourceType: 'ValueSet', + id: 'dinner-vs', + name: 'DinnerVS', + url: 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', + status: 'draft', + compose: { + include: [ + { + valueSet: [ + 'http://food.org/food/ValueSet/hot-food', + 'http://food.org/food/ValueSet/cold-food', + 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', + 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', + 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs' + ] + } + ] + } + }); + expect(loggerSpy.getAllMessages('error')).toHaveLength(1); + expect(loggerSpy.getLastMessage('error')).toBe('Value set with id dinner-vs has component rule with self referencing value sets (by id, value set name, or url). Skipping rule.'); + }); + it('should export a value set that includes a concept component with at least one concept', () => { const valueSet = new FshValueSet('DinnerVS'); const component = new ValueSetConceptComponentRule(true); From 80a54b5f3c8a1e6f1a81a50337d2c35fd06325c4 Mon Sep 17 00:00:00 2001 From: kjefferson Date: Wed, 7 Aug 2024 17:15:25 -0400 Subject: [PATCH 2/7] moving self reference check to setCompose --- src/export/ValueSetExporter.ts | 26 ++++++++------------------ test/export/ValueSetExporter.test.ts | 7 ++----- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts index bcddcf50d..532163cf0 100644 --- a/src/export/ValueSetExporter.ts +++ b/src/export/ValueSetExporter.ts @@ -86,6 +86,14 @@ export class ValueSetExporter { composeElement.valueSet = component.from.valueSets.map(vs => { return this.fisher.fishForMetadata(vs, Type.ValueSet)?.url ?? vs; }); + composeElement.valueSet = composeElement.valueSet.filter(vs => { + if (vs == valueSet.url) { + logger.error( + `Value set with id ${valueSet.id} has component rule with self referencing value set (by id, value set name, or url). Skipping rule.` + ); + }; + return vs != valueSet.url + }); composeElement.valueSet.forEach(vs => { // Canonical URI may include | to specify version: https://www.hl7.org/fhir/references.html#canonical if (!isUri(vs.split('|')[0])) { @@ -448,24 +456,6 @@ export class ValueSetExporter { ); } - // check value sets to ensure not self referencing - let selfReferencedIncludeValueSets = vs?.compose?.include?.[0]?.valueSet?.filter((valueSetItem) => valueSetItem == vs.url) ?? []; - let selfReferencedExcludeValueSets = vs?.compose?.exclude?.[0]?.valueSet?.filter((valueSetItem) => valueSetItem == vs.url) ?? []; - - if ( selfReferencedIncludeValueSets.length > 0 || selfReferencedExcludeValueSets.length > 0 ) { - logger.error( - `Value set with id ${vs.id} has component rule with self referencing value sets (by id, value set name, or url). Skipping rule.` - ) - // remove the self referenced rules for the output - // if (selfReferencedIncludeValueSets.length > 0 ) { - // let nonSelfReferenceValueSets = vs.compose.include[0].valueSet.filter((valueSetItem) => valueSetItem != vs.url) - // vs.compose.include[0].valueSet = nonSelfReferenceValueSets; - // } - // if (selfReferencedExcludeValueSets.length > 0 ) { - // let nonSelfReferenceValueSets = vs.compose.exclude[0].valueSet.filter((valueSetItem) => valueSetItem != vs.url) - // vs.compose.exclude[0].valueSet = nonSelfReferenceValueSets; - // } - } cleanResource(vs, (prop: string) => ['_sliceName', '_primitive'].includes(prop)); this.pkg.valueSets.push(vs); diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts index 5161c3140..0e9f3d8f4 100644 --- a/test/export/ValueSetExporter.test.ts +++ b/test/export/ValueSetExporter.test.ts @@ -501,16 +501,13 @@ describe('ValueSetExporter', () => { { valueSet: [ 'http://food.org/food/ValueSet/hot-food', - 'http://food.org/food/ValueSet/cold-food', - 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', - 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', - 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs' + 'http://food.org/food/ValueSet/cold-food' ] } ] } }); - expect(loggerSpy.getAllMessages('error')).toHaveLength(1); + expect(loggerSpy.getAllMessages('error')).toHaveLength(3); expect(loggerSpy.getLastMessage('error')).toBe('Value set with id dinner-vs has component rule with self referencing value sets (by id, value set name, or url). Skipping rule.'); }); From a95167c0bdb5dc33b82c34ac8f34ce4cbd6047ce Mon Sep 17 00:00:00 2001 From: kjefferson Date: Wed, 7 Aug 2024 17:19:47 -0400 Subject: [PATCH 3/7] typo in error message --- test/export/ValueSetExporter.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts index 0e9f3d8f4..723fa9677 100644 --- a/test/export/ValueSetExporter.test.ts +++ b/test/export/ValueSetExporter.test.ts @@ -508,7 +508,7 @@ describe('ValueSetExporter', () => { } }); expect(loggerSpy.getAllMessages('error')).toHaveLength(3); - expect(loggerSpy.getLastMessage('error')).toBe('Value set with id dinner-vs has component rule with self referencing value sets (by id, value set name, or url). Skipping rule.'); + expect(loggerSpy.getLastMessage('error')).toBe('Value set with id dinner-vs has component rule with self referencing value set (by id, value set name, or url). Skipping rule.'); }); it('should export a value set that includes a concept component with at least one concept', () => { From ec1e9703338b002badd7beffb3ab514cb89fce33 Mon Sep 17 00:00:00 2001 From: kjefferson Date: Tue, 27 Aug 2024 09:05:50 -0400 Subject: [PATCH 4/7] linting files --- src/export/ValueSetExporter.ts | 13 ++++++------- test/export/ValueSetExporter.test.ts | 6 ++++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts index 532163cf0..bdce9b2d8 100644 --- a/src/export/ValueSetExporter.ts +++ b/src/export/ValueSetExporter.ts @@ -87,12 +87,12 @@ export class ValueSetExporter { return this.fisher.fishForMetadata(vs, Type.ValueSet)?.url ?? vs; }); composeElement.valueSet = composeElement.valueSet.filter(vs => { - if (vs == valueSet.url) { - logger.error( - `Value set with id ${valueSet.id} has component rule with self referencing value set (by id, value set name, or url). Skipping rule.` - ); - }; - return vs != valueSet.url + if (vs == valueSet.url) { + logger.error( + `Value set with id ${valueSet.id} has component rule with self referencing value set (by id, value set name, or url). Skipping rule.` + ); + } + return vs != valueSet.url; }); composeElement.valueSet.forEach(vs => { // Canonical URI may include | to specify version: https://www.hl7.org/fhir/references.html#canonical @@ -456,7 +456,6 @@ export class ValueSetExporter { ); } - cleanResource(vs, (prop: string) => ['_sliceName', '_primitive'].includes(prop)); this.pkg.valueSets.push(vs); this.pkg.fshMap.set(vs.getFileName(), { diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts index 723fa9677..f29a3fdf3 100644 --- a/test/export/ValueSetExporter.test.ts +++ b/test/export/ValueSetExporter.test.ts @@ -475,7 +475,7 @@ describe('ValueSetExporter', () => { it('should log error when exporting a value set that includes a component from a self referencing value set', () => { const valueSet = new FshValueSet('DinnerVS'); - valueSet.id = "dinner-vs" + valueSet.id = 'dinner-vs'; const component = new ValueSetConceptComponentRule(true); component.from = { valueSets: [ @@ -508,7 +508,9 @@ describe('ValueSetExporter', () => { } }); expect(loggerSpy.getAllMessages('error')).toHaveLength(3); - expect(loggerSpy.getLastMessage('error')).toBe('Value set with id dinner-vs has component rule with self referencing value set (by id, value set name, or url). Skipping rule.'); + expect(loggerSpy.getLastMessage('error')).toBe( + 'Value set with id dinner-vs has component rule with self referencing value set (by id, value set name, or url). Skipping rule.' + ); }); it('should export a value set that includes a concept component with at least one concept', () => { From f43ef8edd8a799bdc007402c9c2fccdd1dee1d17 Mon Sep 17 00:00:00 2001 From: kjefferson Date: Thu, 5 Sep 2024 12:04:33 -0400 Subject: [PATCH 5/7] update to self reference error message --- src/export/ValueSetExporter.ts | 2 +- test/export/ValueSetExporter.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts index bdce9b2d8..61c76affb 100644 --- a/src/export/ValueSetExporter.ts +++ b/src/export/ValueSetExporter.ts @@ -89,7 +89,7 @@ export class ValueSetExporter { composeElement.valueSet = composeElement.valueSet.filter(vs => { if (vs == valueSet.url) { logger.error( - `Value set with id ${valueSet.id} has component rule with self referencing value set (by id, value set name, or url). Skipping rule.` + `Value set with id ${valueSet.id} has component rule with self-referencing value set (by id, value set name, or url). Removing self-reference.` ); } return vs != valueSet.url; diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts index f29a3fdf3..ce9b28d26 100644 --- a/test/export/ValueSetExporter.test.ts +++ b/test/export/ValueSetExporter.test.ts @@ -509,7 +509,7 @@ describe('ValueSetExporter', () => { }); expect(loggerSpy.getAllMessages('error')).toHaveLength(3); expect(loggerSpy.getLastMessage('error')).toBe( - 'Value set with id dinner-vs has component rule with self referencing value set (by id, value set name, or url). Skipping rule.' + 'Value set with id dinner-vs has component rule with self-referencing value set (by id, value set name, or url). Removing self-reference.' ); }); From c3b1785162be8089a414332dd8cc9d192fe7a18e Mon Sep 17 00:00:00 2001 From: kjefferson Date: Fri, 6 Sep 2024 13:11:38 -0400 Subject: [PATCH 6/7] remove self reference null values --- src/export/ValueSetExporter.ts | 4 +++- test/export/ValueSetExporter.test.ts | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts index 61c76affb..aacf9fd94 100644 --- a/src/export/ValueSetExporter.ts +++ b/src/export/ValueSetExporter.ts @@ -184,7 +184,9 @@ export class ValueSetExporter { this.addConceptComposeElement(composeElement, valueSet.compose.include); } } else { - valueSet.compose.include.push(composeElement); + if (composeElement.valueSet?.length != 0) { + valueSet.compose.include.push(composeElement); + } } } else { this.addConceptComposeElement(composeElement, valueSet.compose.exclude); diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts index ce9b28d26..f146da8de 100644 --- a/test/export/ValueSetExporter.test.ts +++ b/test/export/ValueSetExporter.test.ts @@ -473,7 +473,7 @@ describe('ValueSetExporter', () => { }); }); - it('should log error when exporting a value set that includes a component from a self referencing value set', () => { + it('should remove and log error when exporting a value set that includes a component from a self referencing value set', () => { const valueSet = new FshValueSet('DinnerVS'); valueSet.id = 'dinner-vs'; const component = new ValueSetConceptComponentRule(true); @@ -486,7 +486,12 @@ describe('ValueSetExporter', () => { 'dinner-vs' ] }; + const component2 = new ValueSetConceptComponentRule(true); + component2.from = { + valueSets: ['DinnerVS', 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', 'dinner-vs'] + }; valueSet.rules.push(component); + valueSet.rules.push(component2); doc.valueSets.set(valueSet.name, valueSet); const exported = exporter.export().valueSets; expect(exported.length).toBe(1); @@ -507,7 +512,7 @@ describe('ValueSetExporter', () => { ] } }); - expect(loggerSpy.getAllMessages('error')).toHaveLength(3); + expect(loggerSpy.getAllMessages('error')).toHaveLength(6); expect(loggerSpy.getLastMessage('error')).toBe( 'Value set with id dinner-vs has component rule with self-referencing value set (by id, value set name, or url). Removing self-reference.' ); From d032ee8b79de95289cfc19b4f9d0d1b1e6bbbbc1 Mon Sep 17 00:00:00 2001 From: kjefferson Date: Mon, 16 Sep 2024 08:57:20 -0400 Subject: [PATCH 7/7] adding conditional feedback --- src/export/ValueSetExporter.ts | 2 +- test/export/ValueSetExporter.test.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts index aacf9fd94..19c1b776e 100644 --- a/src/export/ValueSetExporter.ts +++ b/src/export/ValueSetExporter.ts @@ -184,7 +184,7 @@ export class ValueSetExporter { this.addConceptComposeElement(composeElement, valueSet.compose.include); } } else { - if (composeElement.valueSet?.length != 0) { + if (composeElement.valueSet?.length !== 0 || composeElement.system != undefined) { valueSet.compose.include.push(composeElement); } } diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts index f146da8de..78baa7b51 100644 --- a/test/export/ValueSetExporter.test.ts +++ b/test/export/ValueSetExporter.test.ts @@ -478,6 +478,7 @@ describe('ValueSetExporter', () => { valueSet.id = 'dinner-vs'; const component = new ValueSetConceptComponentRule(true); component.from = { + system: 'http://food.org/food1', valueSets: [ 'http://food.org/food/ValueSet/hot-food', 'http://food.org/food/ValueSet/cold-food', @@ -488,6 +489,7 @@ describe('ValueSetExporter', () => { }; const component2 = new ValueSetConceptComponentRule(true); component2.from = { + system: 'http://food.org/food2', valueSets: ['DinnerVS', 'http://hl7.org/fhir/us/minimal/ValueSet/dinner-vs', 'dinner-vs'] }; valueSet.rules.push(component); @@ -504,10 +506,14 @@ describe('ValueSetExporter', () => { compose: { include: [ { + system: 'http://food.org/food1', valueSet: [ 'http://food.org/food/ValueSet/hot-food', 'http://food.org/food/ValueSet/cold-food' ] + }, + { + system: 'http://food.org/food2' } ] }