Repository: FHIR/sushi
Pull request #495 (2020-06-17 13:26:30)

Contained resources in StructureDefinitions, export primitives with children
Completes task [CIMPL-397](https://standardhealthrecord.atlassian.net/browse/CIMPL-397). See also #457 

Primitive elements can have child elements. They were already being added during import, but were getting lost during export in the `toJSON` method.

StructureDefinitions can contain resources on their `contained` element. This element can be accessed using a CaretValueRule. The current implementation checks to see if the element's type is `Resource`, but this may be too restrictive in cases that I haven't thought of yet. Because the resource to add may be an inline Instance, these rules are deferred until after the InstanceExporter finishes.

ngfreiter: One other thing I am noticing is that if you put a CodeableConcept in place of a Resource like so:
```
Instance: MyCodeable
InstanceOf: CodeableConcept
Usage: #inline

Profile: MyProf
Parent: Patient
* ^contained[0] = MyCodeable
```
Then you get a TypeError:
```
 TypeError: Cannot read property 'sdType' of undefined
```

I think this is because in the `checkFixInlineInstance` function on ElementDefinition, we search into the instanceMeta, but the instanceMeta is stripped out when you use `fishForFHIR`.
mint-thompson: Thanks for the detailed comments and for pointing out the case with an Instance of CodeableConcept!
mint-thompson: @ngfreiter After some additional exploration and research, I'm not so sure about the case you pointed out with an Instance of CodeableConcept. A CodeableConcept is not a Resource, so I don't think it could be assigned there? Regardless, I should be doing a bit better than a TypeError in that case.
ngfreiter: Ah yes I agree, it should be an error about setting CodeableConcept onto a type that does not match, but it should not be allowed.

Patch:
diff --git a/src/export/FHIRExporter.ts b/src/export/FHIRExporter.ts
index 1942927..3763e70 100644
--- a/src/export/FHIRExporter.ts
+++ b/src/export/FHIRExporter.ts
@@ -40,6 +40,7 @@ export class FHIRExporter {
     this.valueSetExporter.export();
     this.codeSystemExporter.export();
     this.instanceExporter.export();
+    this.structureDefinitionExporter.applyDeferredRules();
     this.mappingExporter.export();
 
     return this.pkg;
diff --git a/src/export/StructureDefinitionExporter.ts b/src/export/StructureDefinitionExporter.ts
index 54fc7e5..a67dd62 100644
--- a/src/export/StructureDefinitionExporter.ts
+++ b/src/export/StructureDefinitionExporter.ts
@@ -2,7 +2,8 @@ import {
   StructureDefinition,
   ElementDefinition,
   ElementDefinitionBindingStrength,
-  idRegex
+  idRegex,
+  InstanceDefinition
 } from '../fhirtypes';
 import { Profile, Extension, Invariant } from '../fshtypes';
 import { FSHTank } from '../import';
@@ -37,6 +38,8 @@ import { Package } from './Package';
  * The operations and structure of both exporters are very similar, so they currently share an exporter.
  */
 export class StructureDefinitionExporter implements Fishable {
+  deferredRules = new Map<StructureDefinition, CaretValueRule[]>();
+
   constructor(
     private readonly tank: FSHTank,
     private readonly pkg: Package,
@@ -173,7 +176,15 @@ export class StructureDefinitionExporter implements Fishable {
             if (rule.path !== '') {
               element.setInstancePropertyByPath(rule.caretPath, rule.value, this);
             } else {
-              structDef.setInstancePropertyByPath(rule.caretPath, rule.value, this);
+              if (rule.isInstance) {
+                if (this.deferredRules.has(structDef)) {
+                  this.deferredRules.get(structDef).push(rule);
+                } else {
+                  this.deferredRules.set(structDef, [rule]);
+                }
+              } else {
+                structDef.setInstancePropertyByPath(rule.caretPath, rule.value, this);
+              }
             }
           } else if (rule instanceof ObeysRule) {
             const invariant = this.tank.fish(rule.invariant, Type.Invariant) as Invariant;
@@ -201,6 +212,40 @@ export class StructureDefinitionExporter implements Fishable {
     }
   }
 
+  applyDeferredRules() {
+    this.deferredRules.forEach((rules, sd) => {
+      for (const rule of rules) {
+        if (typeof rule.value === 'string') {
+          const fishedValue = this.fishForFHIR(rule.value);
+          // an inline instance will fish up an InstanceDefinition, which can be used directly.
+          // other instances will fish up an Object, which needs to be turned into an InstanceDefinition.
+          // an InstanceDefinition of a resource needs a resourceType, so check for that property.
+          // if we can't find a resourceType or an sdType, we have a non-resource Instance, which is no good
+          if (fishedValue) {
+            if (fishedValue instanceof InstanceDefinition && fishedValue.resourceType) {
+              try {
+                sd.setInstancePropertyByPath(rule.caretPath, fishedValue, this);
+              } catch (e) {
+                logger.error(e, rule.sourceInfo);
+              }
+            } else if (fishedValue instanceof Object && fishedValue.resourceType) {
+              const fishedInstance = InstanceDefinition.fromJSON(fishedValue);
+              try {
+                sd.setInstancePropertyByPath(rule.caretPath, fishedInstance, this);
+              } catch (e) {
+                logger.error(e, rule.sourceInfo);
+              }
+            } else {
+              logger.error(`Could not find a resource named ${rule.value}`, rule.sourceInfo);
+            }
+          } else {
+            logger.error(`Could not find a resource named ${rule.value}`, rule.sourceInfo);
+          }
+        }
+      }
+    });
+  }
+
   /**
    * Handles a ContainsRule that is on an extension path, appropriately exporting it as a reference to a standalone
    * extension or an inline extension.
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index ad01ba4..fd29466 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -335,6 +335,12 @@ export class StructureDefinition {
         // @ts-ignore
         j[prop] = cloneDeep(this[prop]);
       }
+      // children of primitive properties are located by an underscore-prefixed property name
+      // @ts-ignore
+      if (this[`_${prop}`] !== undefined) {
+        // @ts-ignore
+        j[`_${prop}`] = cloneDeep(this[`_${prop}`]);
+      }
     }
 
     // Now handle snapshot and differential
diff --git a/src/fshtypes/rules/CaretValueRule.ts b/src/fshtypes/rules/CaretValueRule.ts
index b9e22bc..754ba08 100644
--- a/src/fshtypes/rules/CaretValueRule.ts
+++ b/src/fshtypes/rules/CaretValueRule.ts
@@ -4,6 +4,7 @@ import { FixedValueType } from './FixedValueRule';
 export class CaretValueRule extends Rule {
   caretPath: string;
   value: FixedValueType;
+  isInstance: boolean;
 
   constructor(path: string) {
     super(path);
diff --git a/src/import/FSHImporter.ts b/src/import/FSHImporter.ts
index 4d50c30..096e65b 100644
--- a/src/import/FSHImporter.ts
+++ b/src/import/FSHImporter.ts
@@ -1232,6 +1232,8 @@ export class FSHImporter extends FSHVisitor {
     // Get the caret path, but slice off the starting ^
     caretValueRule.caretPath = this.visitCaretPath(ctx.caretPath()).slice(1);
     caretValueRule.value = this.visitValue(ctx.value());
+    caretValueRule.isInstance =
+      ctx.value().SEQUENCE() != null && !this.allAliases.has(ctx.value().SEQUENCE().getText());
     return caretValueRule;
   }
 


Test patch:
diff --git a/test/export/FHIRExporter.test.ts b/test/export/FHIRExporter.test.ts
index ae57b7c..64aeb19 100644
--- a/test/export/FHIRExporter.test.ts
+++ b/test/export/FHIRExporter.test.ts
@@ -1,7 +1,11 @@
-import { exportFHIR, Package } from '../../src/export';
-import { FSHTank } from '../../src/import';
-import { FHIRDefinitions } from '../../src/fhirdefs';
+import path from 'path';
+import { exportFHIR, Package, FHIRExporter } from '../../src/export';
+import { FSHTank, FSHDocument } from '../../src/import';
+import { FHIRDefinitions, loadFromPath } from '../../src/fhirdefs';
 import { minimalConfig } from '../utils/minimalConfig';
+import { Instance, Profile } from '../../src/fshtypes';
+import { CaretValueRule } from '../../src/fshtypes/rules';
+import { TestFisher, loggerSpy } from '../testhelpers';
 
 describe('FHIRExporter', () => {
   it('should output empty results with empty input', () => {
@@ -20,4 +24,108 @@ describe('FHIRExporter', () => {
       })
     );
   });
+
+  describe('#containedResources', () => {
+    let defs: FHIRDefinitions;
+    let doc: FSHDocument;
+    let exporter: FHIRExporter;
+
+    beforeAll(() => {
+      defs = new FHIRDefinitions();
+      loadFromPath(
+        path.join(__dirname, '..', 'testhelpers', 'testdefs', 'package'),
+        'testPackage',
+        defs
+      );
+    });
+
+    beforeEach(() => {
+      loggerSpy.reset();
+      doc = new FSHDocument('fileName');
+      const input = new FSHTank([doc], minimalConfig);
+      const pkg = new Package(input.config);
+      const fisher = new TestFisher(input, defs, pkg);
+      exporter = new FHIRExporter(input, pkg, fisher);
+    });
+
+    it('should allow a profile to contain a defined FHIR resource', () => {
+      const profile = new Profile('ContainingProfile');
+      const caretValueRule = new CaretValueRule('');
+      caretValueRule.caretPath = 'contained';
+      caretValueRule.value = 'allergyintolerance-clinical';
+      caretValueRule.isInstance = true;
+      profile.rules.push(caretValueRule);
+      doc.profiles.set(profile.name, profile);
+
+      const result = exporter.export();
+
+      expect(result.profiles.length).toBe(1);
+      expect(result.profiles[0].contained.length).toBe(1);
+      const containedResource = result.profiles[0].contained[0];
+      expect(containedResource).toEqual(
+        defs.allValueSets().find(vs => vs.id === 'allergyintolerance-clinical')
+      );
+    });
+
+    it('should allow a profile to contain a FSH resource', () => {
+      const instance = new Instance('myObservation');
+      instance.instanceOf = 'Observation';
+      doc.instances.set(instance.name, instance);
+
+      const profile = new Profile('ContainingProfile');
+      const caretValueRule = new CaretValueRule('');
+      caretValueRule.caretPath = 'contained';
+      caretValueRule.value = 'myObservation';
+      caretValueRule.isInstance = true;
+      profile.rules.push(caretValueRule);
+      doc.profiles.set(profile.name, profile);
+
+      const result = exporter.export();
+
+      expect(result.profiles.length).toBe(1);
+      expect(result.profiles[0].contained.length).toBe(1);
+      expect(result.profiles[0].contained[0]).toEqual({
+        resourceType: 'Observation',
+        id: 'myObservation'
+      });
+    });
+
+    it('should log an error when a profile tries to contain an instance that is not a resource', () => {
+      const instance = new Instance('MyCodeable');
+      instance.instanceOf = 'CodeableConcept';
+      instance.usage = 'Inline';
+      doc.instances.set(instance.name, instance);
+
+      const profile = new Profile('ContainingProfile');
+      profile.parent = 'Patient';
+      const caretValueRule = new CaretValueRule('');
+      caretValueRule.caretPath = 'contained[0]';
+      caretValueRule.value = 'MyCodeable';
+      caretValueRule.isInstance = true;
+      profile.rules.push(caretValueRule);
+      doc.profiles.set(profile.name, profile);
+
+      const result = exporter.export();
+      expect(result.profiles.length).toBe(1);
+      expect(result.profiles[0].contained).toBeUndefined();
+      expect(loggerSpy.getLastMessage('error')).toMatch(
+        /Could not find a resource named MyCodeable/s
+      );
+    });
+
+    it('should log an error when a profile tries to contain a resource that does not exist', () => {
+      const profile = new Profile('ContainingProfile');
+      const caretValueRule = new CaretValueRule('');
+      caretValueRule.caretPath = 'contained';
+      caretValueRule.value = 'oops-no-resource';
+      caretValueRule.isInstance = true;
+      profile.rules.push(caretValueRule);
+      doc.profiles.set(profile.name, profile);
+
+      exporter.export();
+      expect(loggerSpy.getLastMessage('error')).toMatch(
+        /Could not find a resource named oops-no-resource/s
+      );
+    });
+  });
 });
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 0c1e612..d3d16e9 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -1793,8 +1793,8 @@ describe('InstanceExporter', () => {
         // * entry ^slicing.discriminator.type = #value
         // * entry contains Patient 0..1
         // * entry[PatientsOnly].resource only Patient
-        // * contained contains PatientOrOrganization 0..1
-        // * contained[PatientOrOrganization] only Patient or Organization
+        // * entry contains PatientOrOrganization 0..1
+        // * entry[PatientOrOrganization] only Patient or Organization
         bundle.rules.push(
           caretRule,
           containsRule,
diff --git a/test/export/StructureDefinition.ProfileExporter.test.ts b/test/export/StructureDefinition.ProfileExporter.test.ts
index 92f03b0..c7d3de9 100644
--- a/test/export/StructureDefinition.ProfileExporter.test.ts
+++ b/test/export/StructureDefinition.ProfileExporter.test.ts
@@ -1,11 +1,12 @@
+import path from 'path';
 import { StructureDefinitionExporter, Package } from '../../src/export';
 import { FSHTank, FSHDocument } from '../../src/import';
 import { FHIRDefinitions, loadFromPath } from '../../src/fhirdefs';
-import { Profile } from '../../src/fshtypes';
+import { Profile, Instance } from '../../src/fshtypes';
 import { loggerSpy } from '../testhelpers/loggerSpy';
 import { TestFisher } from '../testhelpers';
-import path from 'path';
 import { minimalConfig } from '../utils/minimalConfig';
+import { CaretValueRule } from '../../src/fshtypes/rules';
 
 describe('ProfileExporter', () => {
   let defs: FHIRDefinitions;
@@ -135,4 +136,25 @@ describe('ProfileExporter', () => {
     expect(exported[1].baseDefinition === exported[0].url);
     expect(exported[2].baseDefinition === exported[1].url);
   });
+
+  it('should defer adding an instance to a profile as a contained resource', () => {
+    const instance = new Instance('myResource');
+    instance.instanceOf = 'Observation';
+    doc.instances.set(instance.name, instance);
+
+    const profile = new Profile('ContainingProfile');
+    const caretValueRule = new CaretValueRule('');
+    caretValueRule.caretPath = 'contained';
+    caretValueRule.value = 'myResource';
+    caretValueRule.isInstance = true;
+    profile.rules.push(caretValueRule);
+    doc.profiles.set(profile.name, profile);
+
+    const exported = exporter.export().profiles;
+    expect(exported.length).toBe(1);
+    expect(exported[0].contained).toBeUndefined();
+    expect(exporter.deferredRules.size).toBe(1);
+    expect(exporter.deferredRules.get(exported[0]).length).toBe(1);
+    expect(exporter.deferredRules.get(exported[0])).toContain(caretValueRule);
+  });
 });
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index ed65bac..8ef7772 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -3853,6 +3853,21 @@ describe('StructureDefinitionExporter', () => {
     ]);
   });
 
+  it('should include the children of primitive elements when serializing to JSON', () => {
+    const profile = new Profile('SpecialUrlId');
+    profile.parent = 'Observation';
+
+    const rule = new CaretValueRule('');
+    rule.caretPath = 'url.id';
+    rule.value = 'my-id';
+    profile.rules.push(rule);
+
+    exporter.exportStructDef(profile);
+    const sd = pkg.profiles[0];
+    const json = sd.toJSON();
+    expect(json).toHaveProperty('_url', { id: 'my-id' });
+  });
+
   it.skip('should not change sliceName based on a CaretValueRule', () => {
     const profile = new Profile('NameChange');
     profile.parent = 'resprate';
diff --git a/test/import/FSHImporter.CodeSystem.test.ts b/test/import/FSHImporter.CodeSystem.test.ts
index 645a16d..5565a52 100644
--- a/test/import/FSHImporter.CodeSystem.test.ts
+++ b/test/import/FSHImporter.CodeSystem.test.ts
@@ -328,7 +328,7 @@ describe('FSHImporter', () => {
           `;
         const result = importSingleText(input);
         const codeSystem = result.codeSystems.get('ZOO');
-        assertCaretValueRule(codeSystem.rules[0], '', 'publisher', 'Matt');
+        assertCaretValueRule(codeSystem.rules[0], '', 'publisher', 'Matt', false);
       });
 
       it('should parse a code system that uses CaretValueRules alongside concepts', () => {
@@ -341,7 +341,7 @@ describe('FSHImporter', () => {
         const codeSystem = result.codeSystems.get('ZOO');
         expect(codeSystem.concepts[0].code).toBe('lion');
         expect(codeSystem.concepts[0].sourceInfo.file).toBe('Zoo.fsh');
-        assertCaretValueRule(codeSystem.rules[0], '', 'publisher', 'Damon');
+        assertCaretValueRule(codeSystem.rules[0], '', 'publisher', 'Damon', false);
       });
 
       it('should log an error when a CaretValueRule contains a path before ^', () => {
diff --git a/test/import/FSHImporter.Extension.test.ts b/test/import/FSHImporter.Extension.test.ts
index 3c45e25..e590fd6 100644
--- a/test/import/FSHImporter.Extension.test.ts
+++ b/test/import/FSHImporter.Extension.test.ts
@@ -318,7 +318,7 @@ describe('FSHImporter', () => {
         const result = importSingleText(input);
         const extension = result.extensions.get('SomeExtension');
         expect(extension.rules).toHaveLength(1);
-        assertCaretValueRule(extension.rules[0], 'id', 'short', 'foo');
+        assertCaretValueRule(extension.rules[0], 'id', 'short', 'foo', false);
       });
     });
 
diff --git a/test/import/FSHImporter.Profile.test.ts b/test/import/FSHImporter.Profile.test.ts
index a1f3daa..4a65c96 100644
--- a/test/import/FSHImporter.Profile.test.ts
+++ b/test/import/FSHImporter.Profile.test.ts
@@ -1443,13 +1443,14 @@ describe('FSHImporter', () => {
         `;
         const result = importSingleText(input);
         const profile = result.profiles.get('ObservationProfile');
-        assertCaretValueRule(profile.rules[0], '', 'description', 'foo');
-        assertCaretValueRule(profile.rules[1], '', 'experimental', false);
+        assertCaretValueRule(profile.rules[0], '', 'description', 'foo', false);
+        assertCaretValueRule(profile.rules[1], '', 'experimental', false, false);
         assertCaretValueRule(
           profile.rules[2],
           '',
           'keyword[0]',
-          new FshCode('bar', 'foo', 'baz').withLocation([6, 25, 6, 37]).withFile('')
+          new FshCode('bar', 'foo', 'baz').withLocation([6, 25, 6, 37]).withFile(''),
+          false
         );
       });
 
@@ -1463,13 +1464,14 @@ describe('FSHImporter', () => {
         `;
         const result = importSingleText(input);
         const profile = result.profiles.get('ObservationProfile');
-        assertCaretValueRule(profile.rules[0], 'status', 'short', 'foo');
-        assertCaretValueRule(profile.rules[1], 'status', 'sliceIsConstraining', false);
+        assertCaretValueRule(profile.rules[0], 'status', 'short', 'foo', false);
+        assertCaretValueRule(profile.rules[1], 'status', 'sliceIsConstraining', false, false);
         assertCaretValueRule(
           profile.rules[2],
           'status',
           'code[0]',
-          new FshCode('bar', 'foo', 'baz').withLocation([6, 29, 6, 41]).withFile('')
+          new FshCode('bar', 'foo', 'baz').withLocation([6, 29, 6, 41]).withFile(''),
+          false
         );
       });
 
@@ -1481,7 +1483,18 @@ describe('FSHImporter', () => {
         `;
         const result = importSingleText(input);
         const profile = result.profiles.get('ObservationProfile');
-        assertCaretValueRule(profile.rules[0], 'status', 'short', 'Non-breaking');
+        assertCaretValueRule(profile.rules[0], 'status', 'short', 'Non-breaking', false);
+      });
+
+      it('should add resources to the contained array using a CaretValueRule', () => {
+        const input = `
+        Profile: ObservationProfile
+        Parent: Observation
+        * ^contained = myResource
+        `;
+        const result = importSingleText(input);
+        const profile = result.profiles.get('ObservationProfile');
+        assertCaretValueRule(profile.rules[0], '', 'contained', 'myResource', true);
       });
     });
 
diff --git a/test/import/FSHImporter.ValueSet.test.ts b/test/import/FSHImporter.ValueSet.test.ts
index 85228c8..0693295 100644
--- a/test/import/FSHImporter.ValueSet.test.ts
+++ b/test/import/FSHImporter.ValueSet.test.ts
@@ -805,7 +805,7 @@ describe('FSHImporter', () => {
           `;
         const result = importSingleText(input);
         const valueSet = result.valueSets.get('ZooVS');
-        assertCaretValueRule(valueSet.rules[0], '', 'publisher', 'foo');
+        assertCaretValueRule(valueSet.rules[0], '', 'publisher', 'foo', false);
       });
 
       it('should parse a value set that uses CaretValueRules alongside components', () => {
@@ -819,7 +819,7 @@ describe('FSHImporter', () => {
         assertValueSetConceptComponent(valueSet.components[0], 'ZOO', undefined, [
           new FshCode('bear', 'ZOO').withLocation([3, 11, 3, 18]).withFile('Simple.fsh')
         ]);
-        assertCaretValueRule(valueSet.rules[0], '', 'publisher', 'foo');
+        assertCaretValueRule(valueSet.rules[0], '', 'publisher', 'foo', false);
       });
 
       it('should log an error when a CaretValueRule contains a path before ^', () => {
diff --git a/test/testhelpers/asserts.ts b/test/testhelpers/asserts.ts
index c249cdb..1c6ea74 100644
--- a/test/testhelpers/asserts.ts
+++ b/test/testhelpers/asserts.ts
@@ -106,13 +106,15 @@ export function assertCaretValueRule(
   rule: Rule,
   path: string,
   caretPath: string,
-  value: FixedValueType
+  value: FixedValueType,
+  isInstance: boolean
 ): void {
   expect(rule).toBeInstanceOf(CaretValueRule);
   const caretValueRule = rule as CaretValueRule;
   expect(caretValueRule.path).toBe(path);
   expect(caretValueRule.caretPath).toBe(caretPath);
   expect(caretValueRule.value).toEqual(value);
+  expect(caretValueRule.isInstance).toBe(isInstance);
 }
 
 export function assertObeysRule(rule: Rule, path: string, invariant: string) {