Repository: FHIR/sushi
Pull request #758 (2021-02-12 22:02:39)

Fix Extension URL on Instance
Fixes https://github.com/FHIR/sushi/issues/751.

The bug described in the issue would happen because `type.extension.url` is an implied rule, and implied rules are applied before explicit rules. So this rule would be applied, but this line:
```
* type = StraumannCSFDAProductCode#NHA "Abutment, Implant, Dental, Endosseous"
```
would overwrite the value of `type` entirely, removing the `type.extension.url` value. To fix this, this PR makes it so that implied rules are applied to one copy of the instance, and explicit rules are applied to another. Then these two objects are merged together. If the implicit and explicit rules conflict, the explicit rules will win, but values that can coexist, like in this bug, are not overwritten.

To test this, you can run the example given in the issue on `master`, and notice that `type.extension[0].url` is not set, and an error is emitted about this. On this branch, that value does get set, and there is no error. You can also try the test on https://github.com/FHIR/sushi/commit/7b003f31ba6f857a9e175a96256b627d57b1a153 and note that it fails, but on the latest commit it will pass.


Patch:
diff --git a/regression/run-regression.ts b/regression/run-regression.ts
index cc66832..1571b3a 100644
--- a/regression/run-regression.ts
+++ b/regression/run-regression.ts
@@ -8,7 +8,7 @@ import program from 'commander';
 import readlineSync from 'readline-sync';
 import extract from 'extract-zip';
 import opener from 'opener';
-import { union } from 'lodash';
+import { isEqual, union } from 'lodash';
 import { createTwoFilesPatch } from 'diff';
 
 // Track temporary files so they are deleted when the process exits
@@ -316,6 +316,12 @@ async function generateDiff(repo: Repo, config: Config, htmlTemplate: string): P
     const v2File = path.join(repoSUSHIDir2, file);
     const [v1Contents, v2Contents] = await Promise.all([readFile(v1File), readFile(v2File)]);
 
+    try {
+      if (isEqual(JSON.parse(v1Contents), JSON.parse(v2Contents))) {
+        continue;
+      }
+    } catch {}
+
     const v1Label = path.relative(config.getRepoDir(repo), v1File);
     const v2Label = path.relative(config.getRepoDir(repo), v2File);
     const patch = createTwoFilesPatch(v1Label, v2Label, v1Contents, v2Contents);
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index a9c3cc7..8c40a11 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -13,7 +13,7 @@ import {
 } from '../fhirtypes/common';
 import { InstanceOfNotDefinedError } from '../errors/InstanceOfNotDefinedError';
 import { Package } from '.';
-import { cloneDeep, uniq } from 'lodash';
+import { cloneDeep, merge, uniq } from 'lodash';
 import { AssignmentRule } from '../fshtypes/rules';
 
 export class InstanceExporter implements Fishable {
@@ -115,8 +115,17 @@ export class InstanceExporter implements Fishable {
     });
 
     const paths = ['', ...rules.map(rule => rule.path)];
+    // To correctly assign properties, we need to:
+    // 1 - Assign implied properties on the original instance
+    // 2 - Assign rule properties on a copy of the result of 1, so that rule assignment can build on implied assignment
+    // 3 - Merge the result of 2 with the result of 1, so that any implied properties which may have been overwrtiten
+    //     in step 2 are maintained...don't worry I'm confused too
     setImpliedPropertiesOnInstance(instanceDef, instanceOfStructureDefinition, paths, this.fisher);
-    ruleMap.forEach(rule => setPropertyOnInstance(instanceDef, rule.pathParts, rule.assignedValue));
+    const ruleInstance = cloneDeep(instanceDef);
+    ruleMap.forEach(rule =>
+      setPropertyOnInstance(ruleInstance, rule.pathParts, rule.assignedValue)
+    );
+    instanceDef = merge(instanceDef, ruleInstance);
     return instanceDef;
   }
 
@@ -292,8 +301,6 @@ export class InstanceExporter implements Fishable {
       instanceDef.meta = { profile: [instanceOfStructureDefinition.url] };
     }
 
-    this.pkg.instances.push(instanceDef);
-
     applyMixinRules(fshDefinition, this.tank);
     // Set Assigned values based on the FSH rules and the Structure Definition
     instanceDef = this.setAssignedValues(fshDefinition, instanceDef, instanceOfStructureDefinition);
@@ -304,6 +311,7 @@ export class InstanceExporter implements Fishable {
       fshDefinition
     );
     cleanResource(instanceDef);
+    this.pkg.instances.push(instanceDef);
 
     // Once all rules are set, we should ensure that we did not add a duplicate profile URL anywhere
     if (instanceDef.meta?.profile) instanceDef.meta.profile = uniq(instanceDef.meta.profile);


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index afb8627..766fe23 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -1582,6 +1582,33 @@ describe('InstanceExporter', () => {
       ]);
     });
 
+    it('should assign a nested sliced extension element that is referred to by name', () => {
+      const fooExtension = new Extension('FooExtension');
+      doc.extensions.set(fooExtension.name, fooExtension);
+      const containsRule = new ContainsRule('maritalStatus.extension');
+      containsRule.items = [{ name: 'foo', type: 'FooExtension' }];
+      patient.rules.push(containsRule);
+      const barRule = new AssignmentRule('maritalStatus.extension[foo].valueString');
+      barRule.value = 'bar';
+      const maritalRule = new AssignmentRule('maritalStatus');
+      maritalRule.value = new FshCode('boo');
+      patientInstance.rules.push(maritalRule, barRule);
+      const exported = exportInstance(patientInstance);
+      expect(exported.maritalStatus).toEqual({
+        extension: [
+          {
+            url: 'http://hl7.org/fhir/us/minimal/StructureDefinition/FooExtension',
+            valueString: 'bar'
+          }
+        ],
+        coding: [
+          {
+            code: 'boo'
+          }
+        ]
+      });
+    });
+
     it('should assign a sliced extension element that is referred to by url', () => {
       const fooExtension = new Extension('FooExtension');
       doc.extensions.set(fooExtension.name, fooExtension);
@@ -2509,9 +2536,7 @@ describe('InstanceExporter', () => {
         // * valueQuantity = MyAge
         respRateInstance.rules.push(inlineRule);
         const exported = exportInstance(respRateInstance);
-        expect(exported.valueQuantity).toEqual({
-          value: 42
-        });
+        expect(exported.valueQuantity.value).toBe(42);
       });
 
       it('should not overwrite the value property when assigning a Quantity object', () => {
@@ -2565,9 +2590,7 @@ describe('InstanceExporter', () => {
         // * valueQuantity = MySimple
         respRateInstance.rules.push(inlineRule);
         const exported = exportInstance(respRateInstance);
-        expect(exported.valueQuantity).toEqual({
-          value: 7
-        });
+        expect(exported.valueQuantity.value).toBe(7);
       });
 
       it('should assign an inline instance of a FSH defined profile of a type to an instance', () => {
@@ -2590,9 +2613,7 @@ describe('InstanceExporter', () => {
         // * valueQuantity = MyQuantity
         respRateInstance.rules.push(inlineRule);
         const exported = exportInstance(respRateInstance);
-        expect(exported.valueQuantity).toEqual({
-          value: 7
-        });
+        expect(exported.valueQuantity.value).toBe(7);
       });
 
       it('should assign an inline instance of an extension to an instance', () => {