Repository: FHIR/sushi (ts)

Issue:
Instance property must be an Array, not an Object
Elements that are defined as 0..* in resources must be JSON arrays in the instance, even if the cardinality has been constrained, or the array contains just one value. It appears array elements are being rendered as Objects rather than Arrays.

Example -- on master branch, at commit `https://github.com/standardhealth/fsh-mcode/commit/4948b6ebe0018ddf2f9d8d8c53a469de972f059d`

Example: `build\input\resources\Condition-mCODEComorbidConditionExample01.json`

IG Publisher reports:  `Condition.category | error | This property must be an Array, not an Object`






Pull request description:
Add validateValueAtPath before setting properties
Fixes #114 

The functionality for putting properties into arrays even if they are constrained is in the `validateValueAtPath` function. We call that before setting properties on instances that are defined by rules. However, we also set properties on instances based on the structure definition. I updated the function `setFixedValuesForDirectChildren` to also call `validateValueAtPath`. This will check the base cardinality and should fix the bug. I added two tests to check the two places the `setFixedValuesForDirectChildren` is called and check that the arrays are properly set.

Patch:
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 3989033..2c41184 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -1,5 +1,5 @@
 import { FSHTank } from '../import/FSHTank';
-import { StructureDefinition, InstanceDefinition, ElementDefinition, PathPart } from '../fhirtypes';
+import { StructureDefinition, InstanceDefinition, ElementDefinition } from '../fhirtypes';
 import { Instance } from '../fshtypes';
 import { logger, Fishable, Type } from '../utils';
 import { setPropertyOnInstance, replaceReferences, replaceField } from '../fhirtypes/common';
@@ -19,6 +19,14 @@ export class InstanceExporter {
     instanceDef: InstanceDefinition,
     instanceOfStructureDefinition: StructureDefinition
   ): InstanceDefinition {
+    // Fix values from the SD for all elements at the top level of the SD
+    this.setFixedValuesForDirectChildren(
+      instanceOfStructureDefinition.findElement(instanceDef.resourceType),
+      '',
+      instanceDef,
+      instanceOfStructureDefinition
+    );
+
     // All rules will be FixValueRule
     fshInstanceDef.rules.forEach(rule => {
       rule = replaceReferences(rule, this.tank, this.fisher);
@@ -29,7 +37,6 @@ export class InstanceExporter {
           this.fisher
         );
 
-        setPropertyOnInstance(instanceDef, pathParts, fixedValue);
         // For each part of that path, we add fixed values from the SD
         let path = '';
         for (const [i, pathPart] of pathParts.entries()) {
@@ -37,20 +44,28 @@ export class InstanceExporter {
           // Add back non-numeric (slice) brackets
           pathPart.brackets?.forEach(b => (path += /^[-+]?\d+$/.test(b) ? '' : `[${b}]`));
           const element = instanceOfStructureDefinition.findElementByPath(path, this.fisher);
-          this.setFixedValuesForDirectChildren(element, pathParts.slice(0, i + 1), instanceDef);
+          // Reconstruct the part of the rule's path that we just got the element for
+          let rulePathPart = rule.path
+            .split('.')
+            .slice(0, i + 1)
+            .join('.');
+          rulePathPart += '.';
+
+          this.setFixedValuesForDirectChildren(
+            element,
+            rulePathPart,
+            instanceDef,
+            instanceOfStructureDefinition
+          );
         }
+
+        // Fix value fom the rule
+        setPropertyOnInstance(instanceDef, pathParts, fixedValue);
       } catch (e) {
         logger.error(e.message, rule.sourceInfo);
       }
     });
 
-    // Fix values from the SD for all elements at the top level of the SD
-    this.setFixedValuesForDirectChildren(
-      instanceOfStructureDefinition.findElement(instanceDef.resourceType),
-      [],
-      instanceDef
-    );
-
     // Remove all _sliceName fields
     replaceField(
       instanceDef,
@@ -71,13 +86,15 @@ export class InstanceExporter {
    * Given an ElementDefinition, set fixed values for the direct children of that element
    * according to the ElementDefinitions of the children
    * @param {ElementDefinition} element - The element whose children we will fix
-   * @param {PathPart[]} existingPath - The path to the element whose children we will fix
+   * @param {string} existingPath - The path to the element whose children we will fix
    * @param {InstanceDefinition} instanceDef - The InstanceDefinition to fix values on
+   * @param {StructureDefinition} instanceOfStructureDefinition - The structure definition the instance instantiates
    */
   private setFixedValuesForDirectChildren(
     element: ElementDefinition,
-    existingPath: PathPart[],
-    instanceDef: InstanceDefinition
+    existingPath: string,
+    instanceDef: InstanceDefinition,
+    instanceOfStructureDefinition: StructureDefinition
   ) {
     const directChildren = element.children(true);
     for (const child of directChildren) {
@@ -87,17 +104,21 @@ export class InstanceExporter {
       );
       if (fixedValueKey) {
         // Get the end of the child path, this is the part that differs from existingPath
-        const childPathPart = {
-          base: child
-            .diffId()
-            .split('.')
-            .slice(-1)[0]
-        };
-        setPropertyOnInstance(
-          instanceDef,
-          [...existingPath, childPathPart],
-          child[fixedValueKey as keyof ElementDefinition]
-        );
+        const childPathPart = child
+          .diffId()
+          .split('.')
+          .slice(-1)[0];
+
+        try {
+          const { fixedValue, pathParts } = instanceOfStructureDefinition.validateValueAtPath(
+            existingPath + childPathPart,
+            child[fixedValueKey as keyof ElementDefinition],
+            this.fisher
+          );
+          setPropertyOnInstance(instanceDef, pathParts, fixedValue);
+        } catch (e) {
+          logger.error(e.message);
+        }
       }
     }
   }
@@ -143,14 +164,12 @@ export class InstanceExporter {
    * @returns {Package}
    */
   export(): Package {
-    for (const doc of this.tank.docs) {
-      for (const instance of doc.instances.values()) {
-        try {
-          const instanceDef = this.exportInstance(instance);
-          this.pkg.instances.push(instanceDef);
-        } catch (e) {
-          logger.error(e.message, e.sourceInfo);
-        }
+    for (const instance of this.tank.getAllInstances()) {
+      try {
+        const instanceDef = this.exportInstance(instance);
+        this.pkg.instances.push(instanceDef);
+      } catch (e) {
+        logger.error(e.message, e.sourceInfo);
       }
     }
     return this.pkg;


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 9f7e2cc..98bf443 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -2,7 +2,7 @@ import { InstanceExporter, Package, StructureDefinitionExporter } from '../../sr
 import { FSHTank, FSHDocument } from '../../src/import';
 import { FHIRDefinitions, loadFromPath } from '../../src/fhirdefs';
 import { Instance, Profile, FshCode, FshReference, Extension } from '../../src/fshtypes';
-import { FixedValueRule, ContainsRule } from '../../src/fshtypes/rules';
+import { FixedValueRule, ContainsRule, CardRule, OnlyRule } from '../../src/fshtypes/rules';
 import { loggerSpy, TestFisher } from '../testhelpers';
 import { InstanceDefinition } from '../../src/fhirtypes';
 import path from 'path';
@@ -185,6 +185,34 @@ describe('InstanceExporter', () => {
       expect(exported.active).toEqual(true);
     });
 
+    it('should fix top level elements to an array even if constrained on the Structure Definition', () => {
+      const condition = new Profile('TestCondition');
+      condition.parent = 'Condition';
+      const cardRule = new CardRule('category');
+      cardRule.min = 1;
+      cardRule.max = '1';
+      condition.rules.push(cardRule);
+      doc.profiles.set(condition.name, condition);
+      const conditionInstance = new Instance('Bar');
+      conditionInstance.instanceOf = 'TestCondition';
+      doc.instances.set(conditionInstance.name, conditionInstance);
+      const fixedValRule = new FixedValueRule('category');
+      const fixedFshCode = new FshCode('foo', 'http://foo.com');
+      fixedValRule.fixedValue = fixedFshCode;
+      condition.rules.push(fixedValRule);
+      const exported = exportInstance(conditionInstance);
+      expect(exported.category).toEqual([
+        {
+          coding: [
+            {
+              code: 'foo',
+              system: 'http://foo.com'
+            }
+          ]
+        }
+      ]);
+    });
+
     it('should fix top level elements that are fixed by a pattern on the Structure Definition', () => {
       const fixedValRule = new FixedValueRule('maritalStatus');
       const fixedFshCode = new FshCode('foo', 'http://foo.com');
@@ -196,6 +224,32 @@ describe('InstanceExporter', () => {
       });
     });
 
+    it('should fix a value onto a elements that are fixed by a pattern on the Structure Definition', () => {
+      const observation = new Profile('TestObservation');
+      observation.parent = 'Observation';
+      doc.profiles.set(observation.name, observation);
+      const onlyRule = new OnlyRule('value[x]');
+      onlyRule.types = [{ type: 'Quantity' }];
+      observation.rules.push(onlyRule); // * value[x] only Quantity
+      const fixedValRule = new FixedValueRule('valueQuantity');
+      const fixedFshCode = new FshCode('foo', 'http://foo.com');
+      fixedValRule.fixedValue = fixedFshCode;
+      observation.rules.push(fixedValRule); // * valueQuantity = foo.com#foo
+      const observationInstance = new Instance('MyObservation');
+      observationInstance.instanceOf = 'TestObservation';
+      const fixedQuantityValue = new FixedValueRule('valueQuantity.value');
+      fixedQuantityValue.fixedValue = 100;
+      observationInstance.rules.push(fixedQuantityValue); // * valueQuantity.value = 100
+      doc.instances.set(observationInstance.name, observationInstance);
+
+      const exported = exportInstance(observationInstance);
+      expect(exported.valueQuantity).toEqual({
+        code: 'foo',
+        system: 'http://foo.com',
+        value: 100
+      });
+    });
+
     it('should fix top level choice elements that are fixed on the Structure Definition', () => {
       const fixedValRule = new FixedValueRule('deceasedBoolean');
       fixedValRule.fixedValue = true;
@@ -301,6 +355,26 @@ describe('InstanceExporter', () => {
       expect(exported.communication).toBeUndefined();
     });
 
+    it('should fix a nested element that has parents defined in the instance and fixed on the SD to an array even if constrained', () => {
+      const cardRule = new CardRule('contact');
+      cardRule.min = 1;
+      cardRule.max = '1';
+      patient.rules.push(cardRule);
+      const fixedValRule = new FixedValueRule('contact.relationship');
+      fixedValRule.fixedValue = new FshCode('mother');
+      patient.rules.push(fixedValRule);
+      const instanceFixedValRule = new FixedValueRule('contact.gender');
+      instanceFixedValRule.fixedValue = new FshCode('foo');
+      instance.rules.push(instanceFixedValRule);
+      const exported = exportInstance(instance);
+      expect(exported.contact).toEqual([
+        {
+          gender: 'foo',
+          relationship: [{ coding: [{ code: 'mother' }] }]
+        }
+      ]);
+    });
+
     // TODO: The fixValue functions should be updated to not fix a value when a parent element sets
     // the value to something different using a pattern
     it.skip('should not fix an element to a value different than a parent pattern value on the Structure Definition', () => {