Check exported resources for presence of multiple choice values (#1537)

* Check exported resources for presence of multiple choice values When a choice element is defined in FHIR, it allows for values of different types with names based on those types. However, any choice element that is present on resource should have only one named value. For example, Observation.value[x] allows for many types, but an Observation resource should not have both valueString and valueInteger. After exporting a resource, check the resource for the presence of multiple values for the same choice element, and log an error when they are detected. As assigned values are validated in exporters, the relevant StructureDefinitions will have their elements unfolded. Keep the same instance of that StructureDefinition that was used for validation so that the unfolded elements are present when performing the multiple choice validation check. * Check exported FHIR for multiple choice values Only one type choice of a choice element should be present on exported FHIR. Check for the presence of multiple type choices. If multiples are found, log an error. Note that it is still fine to apply rules to different type choice elements on a Profile, since those are two separate element definitions in the snapshot and differential lists.
FHIR · Dec 23, 2024 · 2bfe6ad · 2bfe6ad
1 parent 786eb03
commit 2bfe6ad
Show file tree

Hide file tree

Showing 9 changed files with 378 additions and 15 deletions.
diff --git a/src/export/CodeSystemExporter.ts b/src/export/CodeSystemExporter.ts
@@ -1,5 +1,5 @@
 import { FSHTank } from '../import/FSHTank';
-import { CodeSystem, CodeSystemConcept, PathPart } from '../fhirtypes';
+import { CodeSystem, CodeSystemConcept, PathPart, StructureDefinition } from '../fhirtypes';
 import {
   setPropertyOnDefinitionInstance,
   applyInsertRules,
@@ -9,7 +9,8 @@ import {
   validateInstanceFromRawValue,
   isExtension,
   replaceReferences,
-  splitOnPathPeriods
+  splitOnPathPeriods,
+  checkForMultipleChoice
 } from '../fhirtypes/common';
 import { FshCodeSystem } from '../fshtypes';
 import { CaretValueRule, ConceptRule } from '../fshtypes/rules';
@@ -104,7 +105,11 @@ export class CodeSystemExporter {
     }
   }
 
-  private setCaretPathRules(codeSystem: CodeSystem, rules: CaretValueRule[]) {
+  private setCaretPathRules(
+    codeSystem: CodeSystem,
+    rules: CaretValueRule[],
+    codeSystemSD: StructureDefinition
+  ) {
     // soft index resolution relies on the rule's path attribute.
     // a CaretValueRule is created with an empty path, so first
     // transform its arrayPath into a path.
@@ -130,7 +135,6 @@ export class CodeSystemExporter {
     // a codesystem is a specific case where the only implied values are going to be extension urls.
     // so, we only need to track rules that involve an extension.
     const ruleMap: Map<string, { pathParts: PathPart[] }> = new Map();
-    const codeSystemSD = codeSystem.getOwnStructureDefinition(this.fisher);
     // in order to validate rules that set values on contained resources, we need to track information from rules
     // that define the types of those resources. those types could be defined by rules on the "resourceType" element,
     // or they could be defined by the existing resource that is being assigned.
@@ -344,14 +348,16 @@ export class CodeSystemExporter {
       return;
     }
     const codeSystem = new CodeSystem();
+    const codeSystemSD = codeSystem.getOwnStructureDefinition(this.fisher);
     this.setMetadata(codeSystem, fshDefinition);
     this.setConcepts(
       codeSystem,
       fshDefinition.rules.filter(rule => rule instanceof ConceptRule) as ConceptRule[]
     );
     this.setCaretPathRules(
       codeSystem,
-      fshDefinition.rules.filter(rule => rule instanceof CaretValueRule) as CaretValueRule[]
+      fshDefinition.rules.filter(rule => rule instanceof CaretValueRule) as CaretValueRule[],
+      codeSystemSD
     );
 
     // check for another code system with the same id
@@ -364,6 +370,7 @@ export class CodeSystemExporter {
     }
 
     cleanResource(codeSystem, (prop: string) => ['_sliceName', '_primitive'].includes(prop));
+    checkForMultipleChoice(fshDefinition, codeSystem, codeSystemSD);
     this.updateCount(codeSystem, fshDefinition);
     this.pkg.codeSystems.push(codeSystem);
     this.pkg.fshMap.set(codeSystem.getFileName(), {

diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
@@ -23,7 +23,8 @@ import {
   createUsefulSlices,
   determineKnownSlices,
   setImpliedPropertiesOnInstance,
-  getMatchingContainedReferenceId
+  getMatchingContainedReferenceId,
+  checkForMultipleChoice
 } from '../fhirtypes/common';
 import { InstanceOfNotDefinedError } from '../errors/InstanceOfNotDefinedError';
 import { AbstractInstanceOfError } from '../errors/AbstractInstanceOfError';
@@ -922,6 +923,7 @@ export class InstanceExporter implements Fishable {
     );
     this.checkForNamelessSlices(fshDefinition, instanceDef, instanceOfStructureDefinition);
     cleanResource(instanceDef);
+    checkForMultipleChoice(fshDefinition, instanceDef, instanceOfStructureDefinition);
     this.pkg.instances.push(instanceDef);
     if (fshDefinition.usage !== 'Inline') {
       this.pkg.fshMap.set(instanceDef.getFileName(), {

diff --git a/src/export/StructureDefinitionExporter.ts b/src/export/StructureDefinitionExporter.ts
@@ -67,7 +67,8 @@ import {
   getAllConcepts,
   TYPE_CHARACTERISTICS_CODE,
   TYPE_CHARACTERISTICS_EXTENSION,
-  LOGICAL_TARGET_EXTENSION
+  LOGICAL_TARGET_EXTENSION,
+  checkForMultipleChoice
 } from '../fhirtypes/common';
 import { Package } from './Package';
 import { isUri } from 'valid-url';
@@ -1548,6 +1549,12 @@ export class StructureDefinitionExporter implements Fishable {
       logger.log(err.severity, err.message, fshDefinition.sourceInfo);
     });
 
+    checkForMultipleChoice(
+      fshDefinition,
+      structDef,
+      structDef.getOwnStructureDefinition(this.fisher)
+    );
+
     // check for another structure definition with the same id
     // see https://www.hl7.org/fhir/resource.html#id
     // the structure definition has already been added to the package, so it's fine if it matches itself

diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts
@@ -2,7 +2,8 @@ import {
   ValueSet,
   ValueSetComposeIncludeOrExclude,
   ValueSetComposeConcept,
-  PathPart
+  PathPart,
+  StructureDefinition
 } from '../fhirtypes';
 import { FSHTank } from '../import/FSHTank';
 import { FshValueSet, FshCode, ValueSetFilterValue, FshCodeSystem, Instance } from '../fshtypes';
@@ -24,7 +25,8 @@ import {
   validateInstanceFromRawValue,
   determineKnownSlices,
   setImpliedPropertiesOnInstance,
-  splitOnPathPeriods
+  splitOnPathPeriods,
+  checkForMultipleChoice
 } from '../fhirtypes/common';
 import { isUri } from 'valid-url';
 import { flatMap, partition, xor } from 'lodash';
@@ -269,11 +271,14 @@ export class ValueSetExporter {
     }
   }
 
-  private setCaretRules(valueSet: ValueSet, rules: CaretValueRule[]) {
+  private setCaretRules(
+    valueSet: ValueSet,
+    rules: CaretValueRule[],
+    valueSetSD: StructureDefinition
+  ) {
     resolveSoftIndexing(rules);
 
     const ruleMap: Map<string, { pathParts: PathPart[] }> = new Map();
-    const valueSetSD = valueSet.getOwnStructureDefinition(this.fisher);
     // in order to validate rules that set values on contained resources, we need to track information from rules
     // that define the types of those resources. those types could be defined by rules on the "resourceType" element,
     // or they could be defined by the existing resource that is being assigned.
@@ -416,10 +421,13 @@ export class ValueSetExporter {
     }
   }
 
-  private setConceptCaretRules(vs: ValueSet, rules: CaretValueRule[]) {
+  private setConceptCaretRules(
+    vs: ValueSet,
+    rules: CaretValueRule[],
+    valueSetSD: StructureDefinition
+  ) {
     resolveSoftIndexing(rules);
     const ruleMap: Map<string, { pathParts: PathPart[]; rule: CaretValueRule }> = new Map();
-    const valueSetSD = vs.getOwnStructureDefinition(this.fisher);
     for (const rule of rules) {
       const splitConcept = rule.pathArray[0].split('#');
       const system = splitConcept[0];
@@ -552,22 +560,23 @@ export class ValueSetExporter {
       return;
     }
     const vs = new ValueSet();
+    const valueSetSD = vs.getOwnStructureDefinition(this.fisher);
     this.setMetadata(vs, fshDefinition);
     const [conceptCaretRules, otherCaretRules] = partition(
       fshDefinition.rules.filter(rule => rule instanceof CaretValueRule) as CaretValueRule[],
       caretRule => {
         return caretRule.pathArray.length > 0;
       }
     );
-    this.setCaretRules(vs, otherCaretRules);
+    this.setCaretRules(vs, otherCaretRules, valueSetSD);
     this.setCompose(
       vs,
       fshDefinition.rules.filter(
         rule => rule instanceof ValueSetComponentRule
       ) as ValueSetComponentRule[]
     );
     conceptCaretRules.forEach(rule => (rule.isCodeCaretRule = true));
-    this.setConceptCaretRules(vs, conceptCaretRules);
+    this.setConceptCaretRules(vs, conceptCaretRules, valueSetSD);
     if (vs.compose && vs.compose.include.length == 0) {
       throw new ValueSetComposeError(fshDefinition.name);
     }
@@ -582,6 +591,7 @@ export class ValueSetExporter {
     }
 
     cleanResource(vs, (prop: string) => ['_sliceName', '_primitive'].includes(prop));
+    checkForMultipleChoice(fshDefinition, vs, valueSetSD);
     this.pkg.valueSets.push(vs);
     this.pkg.fshMap.set(vs.getFileName(), {
       ...fshDefinition.sourceInfo,

diff --git a/src/fhirtypes/common.ts b/src/fhirtypes/common.ts
@@ -1647,3 +1647,65 @@ export function getMatchingContainedReferenceId(
     }
   }
 }
+
+export function checkForMultipleChoice(
+  fshDef: Profile | Extension | Logical | Resource | FshCodeSystem | FshValueSet | Instance,
+  fhirDef: { [key: string]: any },
+  structDef: StructureDefinition
+) {
+  checkChildrenForMultipleChoice(fshDef, fhirDef, structDef.elements[0]);
+}
+
+function checkChildrenForMultipleChoice(
+  fshDef: Profile | Extension | Logical | Resource | FshCodeSystem | FshValueSet | Instance,
+  instance: { [key: string]: any },
+  element: ElementDefinition
+) {
+  const children = element.children(true);
+  children.forEach(child => {
+    // does this child represent a choice element, such as value[x]?
+    // if so, check for choices
+    if (child.id.endsWith('[x]')) {
+      // get the element names for each type choice
+      const idStart = splitOnPathPeriods(child.id).slice(-1)[0].slice(0, -3);
+      const availableChoices = child.type.map(edType => `${idStart}${upperFirst(edType.code)}`);
+      if (availableChoices.length > 1) {
+        const existingChoices = availableChoices.filter(choice => {
+          return instance[choice] != null || instance[`_${choice}`] != null;
+        });
+        if (existingChoices.length > 1) {
+          logger.error(
+            `${fshDef.name} contains multiple choice value assignments for choice element ${child.id}.`,
+            fshDef.sourceInfo
+          );
+        }
+      }
+    }
+    // does the instance have an object value for this element?
+    // if so, recursively check that object.
+    // since there may also be children of primitives, also check underscore properties
+    const childPathEnd = child.path.split('.').slice(-1)[0];
+    if (instance[childPathEnd] != null && typeof instance[childPathEnd] === 'object') {
+      if (Array.isArray(instance[childPathEnd])) {
+        instance[childPathEnd].forEach((childProperty: any) => {
+          if (childProperty != null && typeof childProperty === 'object') {
+            checkChildrenForMultipleChoice(fshDef, childProperty, child);
+          }
+        });
+      } else {
+        checkChildrenForMultipleChoice(fshDef, instance[childPathEnd], child);
+      }
+    }
+    if (instance[`_${childPathEnd}`] != null && typeof instance[`_${childPathEnd}`] === 'object') {
+      if (Array.isArray(instance[`_${childPathEnd}`])) {
+        instance[`_${childPathEnd}`].forEach((childProperty: any) => {
+          if (childProperty != null && typeof childProperty === 'object') {
+            checkChildrenForMultipleChoice(fshDef, childProperty, child);
+          }
+        });
+      } else {
+        checkChildrenForMultipleChoice(fshDef, instance[`_${childPathEnd}`], child);
+      }
+    }
+  });
+}
diff --git a/test/export/CodeSystemExporter.test.ts b/test/export/CodeSystemExporter.test.ts
@@ -1211,6 +1211,36 @@ describe('CodeSystemExporter', () => {
     });
   });
 
+  it('should output an error when a choice element has values assigned to more than one choice type', () => {
+    const codeSystem = new FshCodeSystem('MultiChoiceSystem')
+      .withFile('MultipleChoice.fsh')
+      .withLocation([3, 4, 8, 24]);
+    const extensionUrl = new CaretValueRule('');
+    extensionUrl.caretPath = 'extension[0].url';
+    extensionUrl.value = 'http://example.org/SomeExt';
+    const extensionString = new CaretValueRule('');
+    extensionString.caretPath = 'extension[0].valueString';
+    extensionString.value = 'multi value';
+    const extensionInteger = new CaretValueRule('');
+    extensionInteger.caretPath = 'extension[0].valueInteger';
+    extensionInteger.value = BigInt(24);
+    const conceptRule = new ConceptRule('bar', 'Bar', 'Bar');
+    codeSystem.rules.push(extensionUrl, extensionString, extensionInteger, conceptRule);
+    doc.codeSystems.set(codeSystem.name, codeSystem);
+
+    const exported = exporter.export().codeSystems;
+    expect(exported.length).toBe(1);
+    expect(exported[0].extension[0]).toEqual({
+      url: 'http://example.org/SomeExt',
+      valueString: 'multi value',
+      valueInteger: 24
+    });
+    expect(loggerSpy.getAllMessages('error')).toHaveLength(1);
+    expect(loggerSpy.getLastMessage('error')).toMatch(
+      /MultiChoiceSystem contains multiple choice value assignments for choice element CodeSystem\.extension\.value\[x\]\..*File: MultipleChoice\.fsh.*Line: 3 - 8\D*/s
+    );
+  });
+
   it('should not override count when ^count is provided by user', () => {
     const codeSystem = new FshCodeSystem('MyCodeSystem');
     const rule = new CaretValueRule('');