Repository: FHIR/sushi
Pull request #1108 (2022-06-22 13:52:57)

Check complex type values when validating assignments
Completes task [CIMPL-969](https://standardhealthrecord.atlassian.net/browse/CIMPL-969).

When validating the a complex type value used in an AssignmentRule, the value may be an InstanceDefinition. Do not assign the value if it would conflict with existing values within the complex type. Assignment of complex values is all-or-nothing: if a conflict exists on part of the complex value, no parts of the complex value are assigned. An error is reported on the line where the Instance is assigned, and the error message refers to the specific value within the Instance that has a conflict.

The test for the "not the first type" is there as a guard against some prior logic involving `type[0].code` that caused problems for me during development. That logic has been removed, but I kept the test in just in case something similar comes up later.

There is some duplicated code for restoring values in `checkAssignInlineInstance`, but I felt that this implementation made more sense than adding new parameters for the existing key and value.


Patch:
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index 8fd781b..ffc0f1e 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -1642,7 +1642,8 @@ export class ElementDefinition {
           stringVal,
           value.toJSON(),
           exactly,
-          value._instanceMeta.sdType ?? value.resourceType
+          value._instanceMeta.sdType ?? value.resourceType,
+          fisher
         );
         break;
       default:
@@ -1685,8 +1686,15 @@ export class ElementDefinition {
    * @throws {ValueAlreadyAssignedError} when the currentElementValue exists and is different than the new value
    * @throws {MismatchedTypeError} when the value does not match the type of the ElementDefinition
    */
-  private assignFHIRValue(fshValue: string, fhirValue: any, exactly: boolean, type: string) {
-    if (this.type[0].code !== type) {
+  private assignFHIRValue(
+    fshValue: string,
+    fhirValue: any,
+    exactly: boolean,
+    type: string,
+    fisher?: Fishable
+  ) {
+    const lineage = fisher ? this.getTypeLineage(type, fisher).map(meta => meta.sdType) : [];
+    if (!this.type.some(t => t.code === type || lineage.includes(t.code))) {
       throw new MismatchedTypeError(type, fshValue, this.type[0].code);
     }
 
@@ -1987,6 +1995,29 @@ export class ElementDefinition {
       metadata => metadata.sdType
     );
     if (this.type?.some(t => lineage.includes(t.code))) {
+      // capture original value to restore after assignment
+      const originalKey = Object.keys(this).find(
+        k => k.startsWith('pattern') || k.startsWith('fixed')
+      ) as keyof ElementDefinition;
+      const originalValue = this[originalKey];
+      // try assigning it to test for value conflicts
+      const stringVal = JSON.stringify(value);
+      this.assignFHIRValue(
+        stringVal,
+        value.toJSON(),
+        true,
+        value._instanceMeta.sdType ?? value.resourceType,
+        fisher
+      );
+      // if the assignment is successful, undo it and return the value
+      const key = Object.keys(this).find(
+        k => k.startsWith('pattern') || k.startsWith('fixed')
+      ) as keyof ElementDefinition;
+      if (key != null) {
+        delete this[key];
+        // @ts-ignore
+        this[originalKey] = originalValue;
+      }
       return value;
     } else {
       // In this case neither the type of the inline instance nor the type of any of its parents matches the
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index 2cd6efa..bba1bad 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -676,16 +676,17 @@ export class StructureDefinition {
 
       previousElement = currentElement;
     }
-    const originalKey = Object.keys(currentElement).find(
-      k => k.startsWith('pattern') || k.startsWith('fixed')
-    ) as keyof ElementDefinition;
-    const originalValue = currentElement[originalKey];
 
     let assignedValue;
     // Assigned resources cannot be assigned by pattern[x]/fixed[x], so we must set assignedValue directly
     if (value instanceof InstanceDefinition) {
+      // checkAssignInlineInstance will throw if it fails
       assignedValue = currentElement.checkAssignInlineInstance(value, fisher).toJSON();
     } else {
+      const originalKey = Object.keys(currentElement).find(
+        k => k.startsWith('pattern') || k.startsWith('fixed')
+      ) as keyof ElementDefinition;
+      const originalValue = currentElement[originalKey];
       // assignValue will throw if it fails, but skip the check if value is null
       if (value != null) {
         // exactly must be true so that we always test assigning with the more strict fixed[x] approach


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index c060033..8c1d7ac 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -4270,6 +4270,14 @@ describe('InstanceExporter', () => {
         // * valueString = "Some Observation"
         doc.instances.set(inlineObservation.name, inlineObservation);
 
+        const inlineOrganization = new Instance('MyInlineOrganization');
+        inlineOrganization.instanceOf = 'Organization';
+        const organizationName = new AssignmentRule('name');
+        organizationName.value = 'Everyone';
+        inlineOrganization.rules.push(organizationName);
+        // * name = "Everyone"
+        doc.instances.set(inlineOrganization.name, inlineOrganization);
+
         const caretRule = new CaretValueRule('entry');
         caretRule.caretPath = 'slicing.discriminator.type';
         caretRule.value = new FshCode('value');
@@ -4384,6 +4392,21 @@ describe('InstanceExporter', () => {
         });
       });
 
+      it('should assign an inline resource that is not the first type to an instance element with a choice type', () => {
+        const bundleValRule = new AssignmentRule('entry[PatientOrOrganization].resource');
+        bundleValRule.value = 'MyInlineOrganization';
+        bundleValRule.isInstance = true;
+        // * entry[PatientOrOrganization].resource = MyInlineOrganization
+        bundleInstance.rules.push(bundleValRule);
+
+        const exported = exportInstance(bundleInstance);
+        expect(exported.entry[0].resource).toEqual({
+          resourceType: 'Organization',
+          id: 'MyInlineOrganization',
+          name: 'Everyone'
+        });
+      });
+
       it('should assign an inline resource to an instance when the resource is not a profile and uses meta', () => {
         // This test reflects a real-world bug reported on Zulip:
         // https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/example.20FSH.20Bundle.20transaction.20with.20Create.20entries
@@ -4715,6 +4738,106 @@ describe('InstanceExporter', () => {
         ]);
       });
 
+      it('should assign an instance that matches existing values', () => {
+        // Profile: TestPatient
+        // Parent: Patient
+        // * name 1..1
+        // * name.family = "Goodweather"
+        // * name.family 1..1
+        // * name.text = "Regular text"
+        const nameCard = new CardRule('name');
+        nameCard.min = 1;
+        nameCard.max = '1';
+        const familyAssignment = new AssignmentRule('name.family');
+        familyAssignment.value = 'Goodweather';
+        const familyCard = new CardRule('name.family');
+        familyCard.min = 1;
+        familyCard.max = '1';
+        const textAssignment = new AssignmentRule('name.text');
+        textAssignment.value = 'Regular text';
+        patient.rules.push(nameCard, familyAssignment, familyCard, textAssignment);
+        // Instance: SameName
+        // InstanceOf: HumanName
+        // Usage: #inline
+        // * text = "Regular text"
+        // * use = #official
+        const sameName = new Instance('SameName');
+        sameName.instanceOf = 'HumanName';
+        sameName.usage = 'Inline';
+        const sameText = new AssignmentRule('text');
+        sameText.value = 'Regular text';
+        const sameUse = new AssignmentRule('use');
+        sameUse.value = new FshCode('official');
+        sameName.rules.push(sameText, sameUse);
+        doc.instances.set(sameName.name, sameName);
+        // Instance: Bar
+        // InstanceOf: TestPatient
+        // * name = SameName
+        const nameAssignment = new AssignmentRule('name')
+          .withFile('Bar.fsh')
+          .withLocation([3, 3, 3, 25]);
+        nameAssignment.value = 'SameName';
+        nameAssignment.isInstance = true;
+        patientInstance.rules.push(nameAssignment);
+        const exported = exportInstance(patientInstance);
+        expect(exported.name[0].family).toBe('Goodweather');
+        expect(exported.name[0].text).toBe('Regular text');
+        expect(exported.name[0].use).toBe('official');
+      });
+
+      it('should log an error when assigning an instance that would overwrite an existing value', () => {
+        // Profile: TestPatient
+        // Parent: Patient
+        // * name 1..1
+        // * name.family = "Goodweather"
+        // * name.family 1..1
+        // * name.text = "Regular text"
+        const nameCard = new CardRule('name');
+        nameCard.min = 1;
+        nameCard.max = '1';
+        const familyAssignment = new AssignmentRule('name.family');
+        familyAssignment.value = 'Goodweather';
+        const familyCard = new CardRule('name.family');
+        familyCard.min = 1;
+        familyCard.max = '1';
+        const textAssignment = new AssignmentRule('name.text');
+        textAssignment.value = 'Regular text';
+        patient.rules.push(nameCard, familyAssignment, familyCard, textAssignment);
+        // Instance: DifferentName
+        // InstanceOf: HumanName
+        // Usage: #inline
+        // * text = "Different text"
+        // * use = #official
+        const differentName = new Instance('DifferentName');
+        differentName.instanceOf = 'HumanName';
+        differentName.usage = 'Inline';
+        const differentText = new AssignmentRule('text');
+        differentText.value = 'Different text';
+        const differentUse = new AssignmentRule('use');
+        differentUse.value = new FshCode('official');
+        differentName.rules.push(differentText, differentUse);
+        doc.instances.set(differentName.name, differentName);
+        // Instance: Bar
+        // InstanceOf: TestPatient
+        // * name = DifferentName
+        const nameAssignment = new AssignmentRule('name')
+          .withFile('Bar.fsh')
+          .withLocation([3, 3, 3, 25]);
+        nameAssignment.value = 'DifferentName';
+        nameAssignment.isInstance = true;
+        patientInstance.rules.push(nameAssignment);
+        const exported = exportInstance(patientInstance);
+        // name.family has a minimum cardinality of 1, so it is set as an implied property
+        expect(exported.name[0].family).toBe('Goodweather');
+        // text is not required, but the assigned Instance's text would violate the Profile, so it is not assigned
+        expect(exported.name[0].text).toBeUndefined();
+        // since the Instance is not assigned, the use is also undefined
+        expect(exported.name[0].use).toBeUndefined();
+        expect(loggerSpy.getLastMessage('error')).toMatch(
+          /Cannot assign Different text to this element.*File: Bar\.fsh.*Line: 3\D*/s
+        );
+      });
+
       it('should assign an instance of a type to an instance and log a warning when the type is not inline', () => {
         const inlineCodeable = new Instance('MyCodeable')
           .withFile('Code.fsh')