Repository: FHIR/sushi (ts)

Issue:
Cannot fix resource to choice element
See this thread for details: https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/contained.20resources.20of.20different.20types.

If the `contained` element is constrained to a choice of several types, such as `Patient` or `Observation`, then we should still allow any resource of type `Patient` or `Observation` to be placed on an instance in the `contained` array. Trying to do this currently causes an error, because we require that an element have a single type if any value is going to be set on it.

Pull request:
Fix contained choice bug
This fixes #377 and the bug that is referenced in the zulip thread linked on #377. It turns out they are actually two separate problems.

We used to be unable to set an element that is a choice of several resources to a specific instance of one of the resources on an Instance. Now if an element on an instance has a choice of several resource types, such as `Patient` or `Observation`, that element can be set to any instance of type `Patient` or `Observation`.

This also fixes a bug with arrays of inline instances. When several inline instances are defined in an array:
```
* contained[0] = medicalHistoryEmpty
* contained[1] = adverseReactionEmpty
* contained[2] = immunizationEmpty
* contained[3] = medicineListEmpty
```
There was a bug with elements in the array beyond the first. The `[0]` index on the first element of the array would be normalized out, so the first path becomes `contained`. Then, because of this bug, we would interpret subsequent elements as children of this path, so we would view them as attempting to override the value of `contained`. This of course caused issues, which are fixed in this PR.

Patch:
diff --git a/src/errors/FixingNonResourceError.ts b/src/errors/FixingNonResourceError.ts
new file mode 100644
index 0000000..0cb520c
--- /dev/null
+++ b/src/errors/FixingNonResourceError.ts
@@ -0,0 +1,7 @@
+export class FixingNonResourceError extends Error {
+  constructor(public resourceType: string, public id: string) {
+    super(
+      `Instance ${id} of type ${resourceType} is not an Instance of a Resource or Profile of a Resource. Only Instances of Resources or Profiles of Resources may be assigned to other Instances.`
+    );
+  }
+}
diff --git a/src/errors/index.ts b/src/errors/index.ts
index 8561321..e12b65a 100644
--- a/src/errors/index.ts
+++ b/src/errors/index.ts
@@ -40,3 +40,4 @@ export * from './InvalidMappingError';
 export * from './InvalidFHIRIdError';
 export * from './ParentDeclaredAsProfileNameError';
 export * from './InvalidResourceTypeError';
+export * from './FixingNonResourceError';
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 56a57e9..5a543dd 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -84,10 +84,7 @@ export class InstanceExporter implements Fishable {
     rules.forEach(rule => {
       try {
         const matchingInlineResourcePaths = inlineResourcePaths.filter(
-          i =>
-            rule.path.startsWith(i.path) &&
-            rule.path !== i.path &&
-            rule.path != `${i.path}.resourceType`
+          i => rule.path.startsWith(`${i.path}.`) && rule.path !== `${i.path}.resourceType`
         );
         // Generate an array of resourceTypes that matches the path, so if path is
         // a.b.c.d.e, and b is a Bundle and D is a Patient,
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index 8fd0ae2..8ca323f 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -27,7 +27,8 @@ import {
   FixedToPatternError,
   MultipleStandardsStatusError,
   InvalidMappingError,
-  InvalidFHIRIdError
+  InvalidFHIRIdError,
+  FixingNonResourceError
 } from '../errors';
 import { setPropertyOnDefinitionInstance, splitOnPathPeriods, isInheritedResource } from './common';
 import { Fishable, Type, Metadata, logger } from '../utils';
@@ -1365,19 +1366,23 @@ export class ElementDefinition {
    * Checks if a resource can be fixed to this element
    * @param {InstanceDefinition} value - The resource to fix
    * @param {Fishable} fisher - A fishable implementation for finding definitions and metadata
-   * @throws {NoSingleTypeError} when the ElementDefinition does not have a single type
    * @throws {MismatchedTypeError} when the ElementDefinition is not of type Resource
+   * @throws {FixingNonResourceError} when the value being fixed is non-Resource
    * @returns {InstanceDefinition} the input value when it can be fixed
    */
   checkFixResource(value: InstanceDefinition, fisher: Fishable): InstanceDefinition {
-    if (!this.hasSingleType()) {
-      throw new NoSingleTypeError('Resource');
-    }
-    const type = this.type[0].code;
-    if (isInheritedResource(value.resourceType, type, fisher)) {
+    if (this.type?.some(t => isInheritedResource(value.resourceType, t.code, fisher))) {
       return value;
     } else {
-      throw new MismatchedTypeError(value.resourceType, value.id, type);
+      const resource = fisher.fishForFHIR(value.resourceType, Type.Resource);
+      if (resource) {
+        // In this case the value is actually a resource, but not one that inherits from this.type
+        const typesString = this.type?.map(t => t.code).join(', ');
+        throw new MismatchedTypeError(value.resourceType, value.id, typesString);
+      } else {
+        // In this case the value is not actually a resource
+        throw new FixingNonResourceError(value.resourceType, value.id);
+      }
     }
   }
 


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 38b03bc..1fefbbd 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -1423,6 +1423,14 @@ describe('InstanceExporter', () => {
         // * active = true
         doc.instances.set(inlineInstance.name, inlineInstance);
 
+        const inlineObservation = new Instance('MyInlineObservation');
+        inlineObservation.instanceOf = 'Observation';
+        const observationValueRule = new FixedValueRule('valueString');
+        observationValueRule.fixedValue = 'Some Observation';
+        inlineObservation.rules.push(observationValueRule);
+        // * valueString = "Some Observation"
+        doc.instances.set(inlineObservation.name, inlineObservation);
+
         const caretRule = new CaretValueRule('entry');
         caretRule.caretPath = 'slicing.discriminator.type';
         caretRule.value = new FshCode('value');
@@ -1433,10 +1441,28 @@ describe('InstanceExporter', () => {
         cardRule.max = '1';
         const typeRule = new OnlyRule('entry[PatientsOnly].resource');
         typeRule.types = [{ type: 'Patient' }];
+
+        const choiceContainsRule = new ContainsRule('entry');
+        choiceContainsRule.items = [{ name: 'PatientOrOrganization' }];
+        const choiceCardRule = new CardRule('entry[PatientOrOrganization]');
+        choiceCardRule.min = 0;
+        choiceCardRule.max = '1';
+        const choiceTypeRule = new OnlyRule('entry[PatientOrOrganization].resource');
+        choiceTypeRule.types = [{ type: 'Patient' }, { type: 'Organization' }];
         // * entry ^slicing.discriminator.type = #value
         // * entry contains Patient 0..1
         // * entry[PatientsOnly].resource only Patient
-        bundle.rules.push(caretRule, containsRule, cardRule, typeRule);
+        // * contained contains PatientOrOrganization 0..1
+        // * contained[PatientOrOrganization] only Patient or Organization
+        bundle.rules.push(
+          caretRule,
+          containsRule,
+          cardRule,
+          typeRule,
+          choiceContainsRule,
+          choiceCardRule,
+          choiceTypeRule
+        );
       });
 
       it('should fix an inline resource to an instance', () => {
@@ -1451,6 +1477,28 @@ describe('InstanceExporter', () => {
         ]);
       });
 
+      it('should fix multiple inline resources to an instance', () => {
+        const inlineRule = new FixedValueRule('contained[0]');
+        inlineRule.fixedValue = 'MyInlinePatient';
+        inlineRule.isResource = true;
+        patientInstance.rules.push(inlineRule); // * contained[0] = MyInlinePatient
+
+        const inlineRule2 = new FixedValueRule('contained[1]');
+        inlineRule2.fixedValue = 'MyInlineObservation';
+        inlineRule2.isResource = true;
+        patientInstance.rules.push(inlineRule2); // * contained[1] = MyInlineObservation
+
+        const exported = exportInstance(patientInstance);
+        expect(exported.contained).toEqual([
+          { resourceType: 'Patient', id: 'MyInlinePatient', active: true },
+          {
+            resourceType: 'Observation',
+            id: 'MyInlineObservation',
+            valueString: 'Some Observation'
+          }
+        ]);
+      });
+
       it('should fix an inline resource to an instance element with a specific type', () => {
         const bundleValRule = new FixedValueRule('entry[PatientsOnly].resource');
         bundleValRule.fixedValue = 'MyInlinePatient';
@@ -1464,6 +1512,43 @@ describe('InstanceExporter', () => {
         });
       });
 
+      it('should fix an inline resource to an instance element with a choice type', () => {
+        const bundleValRule = new FixedValueRule('entry[PatientOrOrganization].resource');
+        bundleValRule.fixedValue = 'MyInlinePatient';
+        bundleValRule.isResource = true;
+        // * entry[PatientOrOrganization].resource = MyInlinePatient
+        bundleInstance.rules.push(bundleValRule);
+
+        const exported = exportInstance(bundleInstance);
+        expect(exported.entry[0].resource).toEqual({
+          resourceType: 'Patient',
+          id: 'MyInlinePatient',
+          active: true
+        });
+      });
+
+      it('should log an error when fixing an inline resource to an invalid choice', () => {
+        const bundleValRule = new FixedValueRule('entry[PatientOrOrganization].resource')
+          .withFile('BadChoice.fsh')
+          .withLocation([1, 2, 3, 4]);
+        bundleValRule.fixedValue = 'MyInlineObservation';
+        bundleValRule.isResource = true;
+        // * entry[PatientOrOrganization].resource = MyInlineObservation
+        bundleInstance.rules.push(bundleValRule);
+
+        const exported = exportInstance(bundleInstance);
+        expect(exported.entry).toBeUndefined();
+        expect(
+          loggerSpy
+            .getAllMessages('error')
+            .some(e =>
+              e.match(
+                /Cannot fix Observation value: MyInlineObservation. Value does not match element type: Patient, Organization/
+              )
+            )
+        ).toBeTruthy();
+      });
+
       it('should log an error when fixing an inline resource that does not exist to an instance', () => {
         const inlineRule = new FixedValueRule('contained[0]')
           .withFile('FakeInstance.fsh')
diff --git a/test/fhirtypes/ElementDefinition.checkFixResource.test.ts b/test/fhirtypes/ElementDefinition.checkFixResource.test.ts
index 0433805..cb27ccf 100644
--- a/test/fhirtypes/ElementDefinition.checkFixResource.test.ts
+++ b/test/fhirtypes/ElementDefinition.checkFixResource.test.ts
@@ -9,6 +9,7 @@ describe('ElementDefinition', () => {
   let defs: FHIRDefinitions;
   let observation: StructureDefinition;
   let inlineInstance: InstanceDefinition;
+  let inlineCodeable: InstanceDefinition;
   let fisher: TestFisher;
 
   beforeAll(() => {
@@ -25,6 +26,10 @@ describe('ElementDefinition', () => {
     inlineInstance = new InstanceDefinition();
     inlineInstance.resourceType = 'Patient';
     inlineInstance.id = 'MyInlineInstance';
+
+    inlineCodeable = new InstanceDefinition();
+    inlineCodeable.resourceType = 'CodeableConcept';
+    inlineCodeable.id = 'MyCodeable';
   });
 
   describe('#checkFixResource', () => {
@@ -34,21 +39,21 @@ describe('ElementDefinition', () => {
       expect(value.resourceType).toBe('Patient');
     });
 
-    it('should throw NoSingleTypeError when element has multiple types', () => {
-      const valueX = observation.elements.find(e => e.id === 'Observation.value[x]');
+    it('should throw MismatchedTypeError when a Resource is fixed on a non-Resource element', () => {
+      const status = observation.elements.find(e => e.id === 'Observation.status');
       expect(() => {
-        valueX.checkFixResource(inlineInstance, fisher);
+        status.checkFixResource(inlineInstance, fisher);
       }).toThrow(
-        'Cannot fix Resource value on this element since this element does not have a single type'
+        'Cannot fix Patient value: MyInlineInstance. Value does not match element type: code'
       );
     });
 
-    it('should throw MismatchedTypeError when the value is fixed to a non-Resource', () => {
-      const status = observation.elements.find(e => e.id === 'Observation.status');
+    it('should throw FixingNonResourceError when a non-Resource value is fixed on a non-Resource', () => {
+      const category = observation.elements.find(e => e.id === 'Observation.category');
       expect(() => {
-        status.checkFixResource(inlineInstance, fisher);
+        category.checkFixResource(inlineCodeable, fisher);
       }).toThrow(
-        'Cannot fix Patient value: MyInlineInstance. Value does not match element type: code'
+        'Instance MyCodeable of type CodeableConcept is not an Instance of a Resource or Profile of a Resource. Only Instances of Resources or Profiles of Resources may be assigned to other Instances.'
       );
     });
   });
diff --git a/test/fhirtypes/StructureDefinition.test.ts b/test/fhirtypes/StructureDefinition.test.ts
index 783f33b..51e5743 100644
--- a/test/fhirtypes/StructureDefinition.test.ts
+++ b/test/fhirtypes/StructureDefinition.test.ts
@@ -1098,6 +1098,15 @@ describe('StructureDefinition', () => {
         const domainRule = new OnlyRule('contained[DomainsOnly]');
         domainRule.types = [{ type: 'DomainResource' }];
         containedDomains.constrainType(domainRule, fisher);
+
+        contained.addSlice('PatientOrObservation');
+        const containedChoice = respRate.findElementByPath(
+          'contained[PatientOrObservation]',
+          fisher
+        );
+        const choiceRule = new OnlyRule('contained[PatientOrObservation]');
+        choiceRule.types = [{ type: 'Patient' }, { type: 'Observation' }];
+        containedChoice.constrainType(choiceRule, fisher);
       });
 
       it('should allow fixing a Patient type InstanceDefinition to a Resource element', () => {
@@ -1129,6 +1138,17 @@ describe('StructureDefinition', () => {
         expect(fixedValue.resourceType).toBe('Patient');
       });
 
+      it('should allow fixing a Patient type InstanceDefinition to a choice element that includes Patient', () => {
+        const instanceDef = new InstanceDefinition();
+        instanceDef.resourceType = 'Patient';
+        const { fixedValue } = respRate.validateValueAtPath(
+          'contained[PatientOrObservation][0]',
+          instanceDef,
+          fisher
+        );
+        expect(fixedValue.resourceType).toBe('Patient');
+      });
+
       it('should allow fixing a Bundle type InstanceDefinition to a Resource element', () => {
         const instanceDef = new InstanceDefinition();
         instanceDef.id = 'OfJoy';
@@ -1155,6 +1175,24 @@ describe('StructureDefinition', () => {
         ).toThrow(/Bundle.*OfJoy.*Patient/);
       });
 
+      it('should not allow fixing a Bundle type InstanceDefinition to a choice element that does not include Bundle', () => {
+        const instanceDef = new InstanceDefinition();
+        instanceDef.id = 'OfJoy';
+        instanceDef.resourceType = 'Bundle';
+        expect(() =>
+          respRate.validateValueAtPath('contained[PatientOrObservation][0]', instanceDef, fisher)
+        ).toThrow(/Bundle.*OfJoy.*Patient, Observation/);
+      });
+
+      it('should not allow fixing a CodeableConcept type InstanceDefinition to any element', () => {
+        const instanceDef = new InstanceDefinition();
+        instanceDef.id = 'CODES';
+        instanceDef.resourceType = 'CodeableConcept';
+        expect(() => respRate.validateValueAtPath('contained[0]', instanceDef, fisher)).toThrow(
+          /CODES.*CodeableConcept is not an Instance of a Resource/
+        );
+      });
+
       // Overriding elements
       it('should allow replacing parts of a Resource element', () => {
         const language = new FshCode('French');