Repository: FHIR/sushi
Pull request #73 (2020-01-06 14:18:23)

Set child elements on primitives
This addresses https://standardhealthrecord.atlassian.net/browse/CIMPL-187, Support for setting IDs and extensions on primitives in instances.

Primitives in FHIR can have child elements, and when a child element is set on a primitive, this is differentiated from setting a value on the primitive by prefixing the name of the primitive with an `_`. For further documentation of how this works, see https://www.hl7.org/fhir/json.html#primitive.

ngfreiter: The latest commit should address both of your comments. I did decide to fix the bug with array elements being added out of order, and added some tests to verify it is working. It also does look like we can check primitiveness just by looking at the first element of the type. Thanks FHIR!

Patch:
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index 63ab681..275fe22 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -383,6 +383,14 @@ export class StructureDefinition {
         if (!pathPart.brackets) pathPart.brackets = [];
         pathPart.brackets.push('0');
       }
+      // Primitive and only primitives have a lower case first letter
+      if (
+        currentElement.type?.length === 1 &&
+        currentElement.type[0].code.charAt(0) ===
+          currentElement.type[0].code.charAt(0).toLowerCase()
+      ) {
+        pathPart.primitive = true;
+      }
     }
     const clone = currentElement.clone();
     // fixValue will throw if it fails
@@ -545,6 +553,7 @@ export type StructureDefinitionContext = {
 export type PathPart = {
   base: string;
   brackets?: string[];
+  primitive?: boolean;
 };
 
 /**
diff --git a/src/fhirtypes/common.ts b/src/fhirtypes/common.ts
index 67562a2..f32437c 100644
--- a/src/fhirtypes/common.ts
+++ b/src/fhirtypes/common.ts
@@ -32,15 +32,23 @@ export function setPropertyOnInstance(
     // instance property here
     let current: any = instance;
     for (const [i, pathPart] of pathParts.entries()) {
-      const key = pathPart.base;
+      // When a primitive has child elements, a _ is appended to the name of the primitive
+      // According to https://www.hl7.org/fhir/json.html#primitive
+      const key =
+        pathPart.primitive && i < pathParts.length - 1 ? `_${pathPart.base}` : pathPart.base;
       // If this part of the path indexes into an array, the index will be the last bracket
       const index = getArrayIndex(pathPart);
       if (index != null) {
         // If the array doesn't exist, create it
         if (current[key] == null) current[key] = [];
         // If the index doesn't exist in the array, add it and lesser indices
-        if (index >= current[key].length) {
-          current[key][index] = {};
+        // Empty elements should be null, not undefined, according to https://www.hl7.org/fhir/json.html#primitive
+        for (let j = 0; j <= index; j++) {
+          if (j < current[key].length && j === index && current[key][index] == null) {
+            current[key][index] = {};
+          } else if (j >= current[key].length) {
+            current[key].push(j === index ? {} : null);
+          }
         }
         // If it isn't the last element, move on, if it is, set the value
         if (i < pathParts.length - 1) {


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 94975b3..3b822ea 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -251,5 +251,58 @@ describe('InstanceExporter', () => {
       instance.rules.push(instanceFixedValRule);
       expect(() => exporter.exportInstance(instance)).toThrow();
     });
+
+    // Fixing children of primitives
+    it('should fix children of primitive values on an instance', () => {
+      const fixedValRule = new FixedValueRule('active.id');
+      fixedValRule.fixedValue = 'foo';
+      instance.rules.push(fixedValRule);
+      doc.instances.set(instance.name, instance);
+      const exported = exporter.exportInstance(instance);
+      expect(exported._active.id).toBe('foo');
+    });
+
+    it('should fix primitive values and their children on an instance', () => {
+      const fixedValRule1 = new FixedValueRule('active');
+      fixedValRule1.fixedValue = true;
+      instance.rules.push(fixedValRule1);
+      const fixedValRule2 = new FixedValueRule('active.id');
+      fixedValRule2.fixedValue = 'foo';
+      instance.rules.push(fixedValRule2);
+      doc.instances.set(instance.name, instance);
+      const exported = exporter.exportInstance(instance);
+      expect(exported.active).toBe(true);
+      expect(exported._active.id).toBe('foo');
+    });
+
+    it('should fix children of primitive value arrays on an instance', () => {
+      const fixedValRule = new FixedValueRule('address[0].line[1].extension[0].url');
+      fixedValRule.fixedValue = 'foo';
+      instance.rules.push(fixedValRule);
+      doc.instances.set(instance.name, instance);
+      const exported = exporter.exportInstance(instance);
+      expect(exported.address.length).toBe(1);
+      expect(exported.address[0]._line.length).toBe(2);
+      expect(exported.address[0]._line[0]).toBeNull();
+      expect(exported.address[0]._line[1].extension.length).toBe(1);
+      expect(exported.address[0]._line[1].extension[0].url).toBe('foo');
+    });
+
+    it('should fix children of primitive value arrays on an instance with out of order rules', () => {
+      const fixedValRule1 = new FixedValueRule('address[0].line[1].extension[0].url');
+      fixedValRule1.fixedValue = 'bar';
+      instance.rules.push(fixedValRule1);
+      const fixedValRule2 = new FixedValueRule('address[0].line[0].extension[0].url');
+      fixedValRule2.fixedValue = 'foo';
+      instance.rules.push(fixedValRule2);
+      doc.instances.set(instance.name, instance);
+      const exported = exporter.exportInstance(instance);
+      expect(exported.address.length).toBe(1);
+      expect(exported.address[0]._line.length).toBe(2);
+      expect(exported.address[0]._line[0].extension.length).toBe(1);
+      expect(exported.address[0]._line[0].extension[0].url).toBe('foo');
+      expect(exported.address[0]._line[1].extension.length).toBe(1);
+      expect(exported.address[0]._line[1].extension[0].url).toBe('bar');
+    });
   });
 });
diff --git a/test/fhirtypes/ElementDefinition.setInstancePropertyByPath.test.ts b/test/fhirtypes/ElementDefinition.setInstancePropertyByPath.test.ts
index 3ec3338..76c8a75 100644
--- a/test/fhirtypes/ElementDefinition.setInstancePropertyByPath.test.ts
+++ b/test/fhirtypes/ElementDefinition.setInstancePropertyByPath.test.ts
@@ -68,7 +68,15 @@ describe('ElementDefinition', () => {
       status.setInstancePropertyByPath('type[2].code', 'foo', resolve);
       expect(status.type.length).toBe(3);
       expect(status.type[2]).toEqual({ code: 'foo' });
-      expect(status.type[1]).toBeUndefined();
+      expect(status.type[1]).toBeNull();
+    });
+
+    it('should add an instance property in an array that has been empty filled', () => {
+      status.setInstancePropertyByPath('type[2].code', 'foo', resolve);
+      status.setInstancePropertyByPath('type[1].code', 'bar', resolve);
+      expect(status.type.length).toBe(3);
+      expect(status.type[2]).toEqual({ code: 'foo' });
+      expect(status.type[1]).toEqual({ code: 'bar' });
     });
 
     it('should change an instance property in an array', () => {
@@ -110,5 +118,35 @@ describe('ElementDefinition', () => {
       expect(status.code[0]).toEqual({ code: 'foo', system: 'http://example.com' });
       expect(status.code[1]).toEqual({ code: 'bar', system: 'http://example.com' });
     });
+
+    // Children of primitives
+    it('should set a child of a primitive instance property which has a value', () => {
+      status.setInstancePropertyByPath('short', 'foo', resolve);
+      status.setInstancePropertyByPath('short.id', 'bar', resolve);
+      expect(status.short).toBe('foo');
+      // @ts-ignore
+      expect(status._short.id).toBe('bar');
+    });
+
+    it('should set a child of a primitive instance property array which has a value', () => {
+      status.setInstancePropertyByPath('alias[0]', 'foo', resolve);
+      status.setInstancePropertyByPath('alias[0].id', 'bar', resolve);
+      expect(status.alias.length).toBe(1);
+      expect(status.alias[0]).toBe('foo');
+      // @ts-ignore
+      expect(status._alias[0].id).toBe('bar');
+    });
+
+    it('should set a child of a primitive instance property array and null fill the array', () => {
+      status.setInstancePropertyByPath('alias[1]', 'foo', resolve);
+      status.setInstancePropertyByPath('alias[1].id', 'bar', resolve);
+      expect(status.alias.length).toBe(2);
+      expect(status.alias[0]).toBeNull();
+      expect(status.alias[1]).toBe('foo');
+      // @ts-ignore
+      expect(status._alias[0]).toBeNull();
+      // @ts-ignore
+      expect(status._alias[1].id).toBe('bar');
+    });
   });
 });
diff --git a/test/fhirtypes/StructureDefinition.test.ts b/test/fhirtypes/StructureDefinition.test.ts
index 563b0b2..4a40ace 100644
--- a/test/fhirtypes/StructureDefinition.test.ts
+++ b/test/fhirtypes/StructureDefinition.test.ts
@@ -346,8 +346,16 @@ describe('StructureDefinition', () => {
       observation.setInstancePropertyByPath('contact[4].telecom[0].value', 'foo', resolve);
       expect(observation.contact.length).toBe(5);
       expect(observation.contact[4]).toEqual({ telecom: [{ value: 'foo' }] });
-      expect(observation.contact[3]).toBeUndefined();
-      expect(observation.contact[2]).toBeUndefined();
+      expect(observation.contact[3]).toBeNull();
+      expect(observation.contact[2]).toBeNull();
+    });
+
+    it('should add an instance property in an array that has been empty filled', () => {
+      observation.setInstancePropertyByPath('contact[3].telecom[0].value', 'foo', resolve);
+      observation.setInstancePropertyByPath('contact[2].telecom[0].value', 'bar', resolve);
+      expect(observation.contact.length).toBe(4);
+      expect(observation.contact[3]).toEqual({ telecom: [{ value: 'foo' }] });
+      expect(observation.contact[2]).toEqual({ telecom: [{ value: 'bar' }] });
     });
 
     it('should change an instance property in an array', () => {
@@ -406,6 +414,36 @@ describe('StructureDefinition', () => {
       });
     });
 
+    // Children of primitives
+    it('should set a child of a primitive instance property which has a value', () => {
+      observation.setInstancePropertyByPath('version', 'foo', resolve);
+      observation.setInstancePropertyByPath('version.id', 'bar', resolve);
+      expect(observation.version).toBe('foo');
+      // @ts-ignore
+      expect(observation._version.id).toBe('bar');
+    });
+
+    it('should set a child of a primitive instance property array which has a value', () => {
+      observation.setInstancePropertyByPath('contextInvariant[0]', 'foo', resolve);
+      observation.setInstancePropertyByPath('contextInvariant[0].id', 'bar', resolve);
+      expect(observation.contextInvariant.length).toBe(1);
+      expect(observation.contextInvariant[0]).toBe('foo');
+      // @ts-ignore
+      expect(observation._contextInvariant[0].id).toBe('bar');
+    });
+
+    it('should set a child of a primitive instance property array and null fill the array', () => {
+      observation.setInstancePropertyByPath('contextInvariant[1]', 'foo', resolve);
+      observation.setInstancePropertyByPath('contextInvariant[1].id', 'bar', resolve);
+      expect(observation.contextInvariant.length).toBe(2);
+      expect(observation.contextInvariant[0]).toBeNull();
+      expect(observation.contextInvariant[1]).toBe('foo');
+      // @ts-ignore
+      expect(observation._contextInvariant[0]).toBeNull();
+      // @ts-ignore
+      expect(observation._contextInvariant[1].id).toBe('bar');
+    });
+
     // Invalid access
     it('should throw an InvalidElementAccessError when trying to access the snapshot', () => {
       expect(() => {
@@ -437,7 +475,7 @@ describe('StructureDefinition', () => {
       const { fixedValue, pathParts } = structureDefinition.validateValueAtPath('version', '4.0.2');
       expect(fixedValue).toBe('4.0.2');
       expect(pathParts.length).toBe(1);
-      expect(pathParts[0]).toEqual({ base: 'version' });
+      expect(pathParts[0]).toEqual({ primitive: true, base: 'version' });
     });
 
     // Invalid paths
@@ -473,7 +511,7 @@ describe('StructureDefinition', () => {
       expect(fixedValue).toBe('foo');
       expect(pathParts.length).toBe(2);
       expect(pathParts[0]).toEqual({ base: 'identifier', brackets: ['0'] });
-      expect(pathParts[1]).toEqual({ base: 'value' });
+      expect(pathParts[1]).toEqual({ primitive: true, base: 'value' });
     });
 
     it('should allow fixing an instance value to an element in an array, with implied 0 index', () => {
@@ -485,7 +523,7 @@ describe('StructureDefinition', () => {
       expect(fixedValue).toBe('foo');
       expect(pathParts.length).toBe(2);
       expect(pathParts[0]).toEqual({ base: 'identifier', brackets: ['0'] });
-      expect(pathParts[1]).toEqual({ base: 'value' });
+      expect(pathParts[1]).toEqual({ primitive: true, base: 'value' });
     });
 
     it('should allow fixing an instance value to an element in an array if the element was constrained from an array', () => {
@@ -499,7 +537,7 @@ describe('StructureDefinition', () => {
       expect(pathParts.length).toBe(3);
       expect(pathParts[0]).toEqual({ base: 'code' });
       expect(pathParts[1]).toEqual({ base: 'coding', brackets: ['RespRateCode', '0'] }); // 0 in path parts means value will be set in an array
-      expect(pathParts[2]).toEqual({ base: 'id' });
+      expect(pathParts[2]).toEqual({ primitive: true, base: 'id' });
     });
 
     it('should not allow using array brackets when an element is not an array', () => {
@@ -535,7 +573,7 @@ describe('StructureDefinition', () => {
       expect(pathParts.length).toBe(3);
       expect(pathParts[0]).toEqual({ base: 'category', brackets: ['VSCat', '0'] });
       expect(pathParts[1]).toEqual({ base: 'coding', brackets: ['0'] });
-      expect(pathParts[2]).toEqual({ base: 'version' });
+      expect(pathParts[2]).toEqual({ primitive: true, base: 'version' });
     });
 
     it('should allow fixing an instance value on a slice array', () => {
@@ -547,7 +585,7 @@ describe('StructureDefinition', () => {
       expect(fixedValue).toBe('foo');
       expect(pathParts.length).toBe(2);
       expect(pathParts[0]).toEqual({ base: 'extension', brackets: ['required', '3'] });
-      expect(pathParts[1]).toEqual({ base: 'value[x]' });
+      expect(pathParts[1]).toEqual({ primitive: true, base: 'value[x]' });
     });
 
     it('should allow setting values directly on extensions by accessing indexes', () => {
@@ -562,7 +600,7 @@ describe('StructureDefinition', () => {
       expect(fixedValue).toBe('foo');
       expect(pathParts.length).toBe(2);
       expect(pathParts[0]).toEqual({ base: 'extension', brackets: ['2'] });
-      expect(pathParts[1]).toEqual({ base: 'url' });
+      expect(pathParts[1]).toEqual({ primitive: true, base: 'url' });
     });
   });