Repository: FHIR/sushi
Pull request #772 (2021-03-01 23:30:21)

Fix issues with extra extensions on instances
This fixes https://github.com/FHIR/sushi/issues/759 which is one single issue which actually encompasses two semi-related issues that result in extra extensions on instances. This PR fixes both of those issues.

The issue has an example demonstrating the first bug in the description, but here is a simpler way to recreate it:
```
Extension: TestString
* extension.value[x] only string
* valueString = "foo"
* valueString 1..1

Profile: TestProfile
Parent: Observation
* extension contains TestString named string 1..1

Instance: TestObs
InstanceOf: TestProfile
* status = #final
* code = #testcode
* extension[TestString].valueString = "foo"
```
If you run this FSH on master, you will see two `TestString` extensions on the `TestObs` instance, where you would really only expect 1. This happened because SUSHI expects this line:
```
* extension[TestString].valueString = "foo"
```
to look more like this:
```
* extension[string].valueString = "foo"
```
when you are referring to the `string` slice which contains the `TestString` extension. By referring to `TestString` directly, the user uses the syntax that in the past was used for adding extensions that are _not_ on the profile. This is fixed by the changes in `validateValueAtPath`. Now when we encounter the situation that an `extension` exists on a profile, but was found using a name that isn't the `sliceName`, we update the path to instead use the `sliceName`, since later processing works off this assumption.

The second bug is also demonstrated at the bottom of the issue, in this comment: https://github.com/FHIR/sushi/issues/759#issuecomment-779403820. The problem here is slightly different, even though it looks similar. When the `MyBooleanExtension` is added on `ChildProfile`, SUSHI has no way of knowing that a `MyBooleanExtension` already exists on that profile, because SUSHI used to rely on the `_sliceName` property to detect this type of thing. Since the rules on `MyProfile` are processed separately from `ChildProfile`, there is no `_sliceName`, and so SUSHI does not detect that that extension is present. This bug is fixed by the changes in `setPropertyOnInstance`, so now that function will detect when an extension has the same `url` as an existing extension on the instance, and adjust the indices accordingly.

Sorry to put it up as one commit and PR, it really is two separate bugs. But since this code is relatively tightly coupled, it was easier for me to just make sure both solutions played nicely. 


Patch:
diff --git a/src/export/CodeSystemExporter.ts b/src/export/CodeSystemExporter.ts
index d83de52..95bf863 100644
--- a/src/export/CodeSystemExporter.ts
+++ b/src/export/CodeSystemExporter.ts
@@ -47,7 +47,7 @@ export class CodeSystemExporter {
           rule.value,
           this.fisher
         );
-        setPropertyOnInstance(codeSystem, pathParts, assignedValue);
+        setPropertyOnInstance(codeSystem, pathParts, assignedValue, this.fisher);
       } catch (e) {
         logger.error(e.message, rule.sourceInfo);
       }
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 8c40a11..1853039 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -123,7 +123,7 @@ export class InstanceExporter implements Fishable {
     setImpliedPropertiesOnInstance(instanceDef, instanceOfStructureDefinition, paths, this.fisher);
     const ruleInstance = cloneDeep(instanceDef);
     ruleMap.forEach(rule =>
-      setPropertyOnInstance(ruleInstance, rule.pathParts, rule.assignedValue)
+      setPropertyOnInstance(ruleInstance, rule.pathParts, rule.assignedValue, this.fisher)
     );
     instanceDef = merge(instanceDef, ruleInstance);
     return instanceDef;
diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts
index 854abe2..f71091a 100644
--- a/src/export/ValueSetExporter.ts
+++ b/src/export/ValueSetExporter.ts
@@ -112,7 +112,7 @@ export class ValueSetExporter {
             rule.value,
             this.fisher
           );
-          setPropertyOnInstance(valueSet, pathParts, assignedValue);
+          setPropertyOnInstance(valueSet, pathParts, assignedValue, this.fisher);
         }
       } catch (e) {
         logger.error(e.message, rule.sourceInfo);
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index 26407f9..48c0479 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -17,7 +17,8 @@ import {
   setPropertyOnDefinitionInstance,
   HasName,
   HasId,
-  isInheritedResource
+  isInheritedResource,
+  isExtension
 } from './common';
 import { Fishable, Type } from '../utils/Fishable';
 import { applyMixins, parseFSHPath, assembleFSHPath } from '../utils';
@@ -494,6 +495,18 @@ export class StructureDefinition {
         }
       }
 
+      // If the element is an extension, and we found that extension via some other identifier than the sliceName
+      // we want to replace the path to use the sliceName, since we can assume that was the user's intent
+      if (
+        isExtension(pathPart.base) &&
+        pathPart.slices &&
+        currentElement?.sliceName &&
+        currentElement?.sliceName !== pathPart.slices.join('/')
+      ) {
+        pathPart.slices = currentElement.sliceName.split('/');
+        pathPart.brackets = [...pathPart.slices, ...pathPart.brackets.filter(b => /^\d+$/.test(b))];
+      }
+
       if (
         !currentElement &&
         pathPart.base === 'resourceType' &&
diff --git a/src/fhirtypes/common.ts b/src/fhirtypes/common.ts
index 7bb2d9f..102a3a6 100644
--- a/src/fhirtypes/common.ts
+++ b/src/fhirtypes/common.ts
@@ -55,7 +55,7 @@ export function setPropertyOnDefinitionInstance(
   const instanceSD = instance.getOwnStructureDefinition(fisher);
   const { assignedValue, pathParts } = instanceSD.validateValueAtPath(path, value, fisher);
   setImpliedPropertiesOnInstance(instance, instanceSD, [path], fisher);
-  setPropertyOnInstance(instance, pathParts, assignedValue);
+  setPropertyOnInstance(instance, pathParts, assignedValue, fisher);
 }
 
 export function setImpliedPropertiesOnInstance(
@@ -116,14 +116,15 @@ export function setImpliedPropertiesOnInstance(
   });
   sdRuleMap.forEach((value, path) => {
     const { pathParts } = instanceOfStructureDefinition.validateValueAtPath(path, null, fisher);
-    setPropertyOnInstance(instanceDef, pathParts, value);
+    setPropertyOnInstance(instanceDef, pathParts, value, fisher);
   });
 }
 
 export function setPropertyOnInstance(
   instance: StructureDefinition | ElementDefinition | InstanceDefinition | ValueSet | CodeSystem,
   pathParts: PathPart[],
-  assignedValue: any
+  assignedValue: any,
+  fisher: Fishable
 ): void {
   if (assignedValue != null) {
     // If we can assign the value on the StructureDefinition StructureDefinition, then we can set the
@@ -150,8 +151,9 @@ export function setPropertyOnInstance(
           }
           const sliceIndices: number[] = [];
           // Find the indices where slices are placed
+          const sliceExtensionUrl = fisher.fishForMetadata(sliceName)?.url;
           current[pathPart.base]?.forEach((el: any, i: number) => {
-            if (el?._sliceName === sliceName) {
+            if (el?._sliceName === sliceName || (el?.url && el?.url === sliceExtensionUrl)) {
               sliceIndices.push(i);
             }
           });
@@ -632,6 +634,10 @@ export class HasName {
   }
 }
 
+export function isExtension(path: string): boolean {
+  return ['modifierExtension', 'extension'].includes(path);
+}
+
 export class HasId {
   id?: FHIRId;
   /**


Test patch:
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index 2ed7dcc..ed0024b 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -3251,6 +3251,52 @@ describe('StructureDefinitionExporter', () => {
     });
   });
 
+  it('should identify existing extensions by URL when applying a CaretValueRule on a StructureDefintiion', () => {
+    const parentProfile = new Profile('Foo');
+    parentProfile.parent = 'Observation';
+
+    const parentRule = new CaretValueRule('');
+    parentRule.caretPath = 'extension[mothersMaidenName].valueString';
+    parentRule.value = 'ParentName';
+    parentProfile.rules.push(parentRule);
+
+    const childProfile = new Profile('Bar');
+    childProfile.parent = 'Foo';
+
+    const childRule = new CaretValueRule('');
+    childRule.caretPath = 'extension[mothersMaidenName].valueString';
+    childRule.value = 'ChildName';
+    childProfile.rules.push(childRule);
+
+    // Profile: Foo
+    // Parent: Observation
+    // * ^extension[mothersMaidenName].valueString = "ParentName"
+    //
+    // Profile: Bar
+    // Parent: Foo
+    // * ^extension[mothersMaidenName].valueString = "ChildName"
+    exporter.exportStructDef(parentProfile);
+    exporter.exportStructDef(childProfile);
+
+    const sd = pkg.profiles[1];
+
+    // We should only find one copy of the extension
+    expect(
+      sd.extension.filter(
+        e => e.url === 'http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName'
+      )
+    ).toHaveLength(1);
+    // And it should have the correct value
+    expect(
+      sd.extension.find(
+        e => e.url === 'http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName'
+      )
+    ).toEqual({
+      url: 'http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName',
+      valueString: 'ChildName'
+    });
+  });
+
   // ObeysRule
   it('should apply an ObeysRule at the specified path', () => {
     const profile = new Profile('Foo');
diff --git a/test/fhirtypes/StructureDefinition.test.ts b/test/fhirtypes/StructureDefinition.test.ts
index 35e7b6d..936b548 100644
--- a/test/fhirtypes/StructureDefinition.test.ts
+++ b/test/fhirtypes/StructureDefinition.test.ts
@@ -1215,8 +1215,8 @@ describe('StructureDefinition', () => {
       expect(pathParts.length).toBe(2);
       expect(pathParts[0]).toEqual({
         base: 'extension',
-        brackets: ['http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName', '0'],
-        slices: ['http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName']
+        brackets: ['patient-mothersMaidenName', '0'],
+        slices: ['patient-mothersMaidenName']
       });
       expect(respRate.elements.length).toBe(originalLength + 5);
     });
@@ -1246,6 +1246,50 @@ describe('StructureDefinition', () => {
       );
     });
 
+    it('should rename extensions when they are referred to by name instead of sliceName', () => {
+      const originalLength = respRate.elements.length;
+      const extension = respRate.findElementByPath('extension', fisher);
+      extension.sliceIt('type', '$this', false, 'open');
+      const slice = extension.addSlice('maiden-name');
+      slice.type[0].profile = ['http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName'];
+
+      const { assignedValue, pathParts } = respRate.validateValueAtPath(
+        'extension[patient-mothersMaidenName].valueString',
+        'foo',
+        fisher
+      );
+      expect(assignedValue).toBe('foo');
+      expect(pathParts.length).toBe(2);
+      expect(pathParts[0]).toEqual({
+        base: 'extension',
+        brackets: ['maiden-name', '0'],
+        slices: ['maiden-name']
+      });
+      expect(respRate.elements.length).toBe(originalLength + 6);
+    });
+
+    it('should rename modifierExtensions when they are referred to by name instead of sliceName', () => {
+      const originalLength = respRate.elements.length;
+      const modExtension = respRate.findElementByPath('modifierExtension', fisher);
+      modExtension.sliceIt('type', '$this', false, 'open');
+      const slice = modExtension.addSlice('maiden-name');
+      slice.type[0].profile = ['http://hl7.org/fhir/StructureDefinition/patient-mothersMaidenName'];
+
+      const { assignedValue, pathParts } = respRate.validateValueAtPath(
+        'modifierExtension[patient-mothersMaidenName].valueString',
+        'foo',
+        fisher
+      );
+      expect(assignedValue).toBe('foo');
+      expect(pathParts.length).toBe(2);
+      expect(pathParts[0]).toEqual({
+        base: 'modifierExtension',
+        brackets: ['maiden-name', '0'],
+        slices: ['maiden-name']
+      });
+      expect(respRate.elements.length).toBe(originalLength + 6);
+    });
+
     describe('#Inline Instances', () => {
       beforeEach(() => {
         const contained = respRate.findElementByPath('contained', fisher);
diff --git a/test/testhelpers/testdefs/package/StructureDefinition-familymemberhistory-type.json b/test/testhelpers/testdefs/package/StructureDefinition-familymemberhistory-type.json
new file mode 100644
index 0000000..0a161c9
--- /dev/null
+++ b/test/testhelpers/testdefs/package/StructureDefinition-familymemberhistory-type.json
@@ -0,0 +1,192 @@
+{
+  "resourceType": "StructureDefinition",
+  "id": "familymemberhistory-type",
+  "extension": [
+    { "url": "http://hl7.org/fhir/StructureDefinition/structuredefinition-wg", "valueCode": "pc" },
+    { "url": "http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm", "valueInteger": 1 }
+  ],
+  "url": "http://hl7.org/fhir/StructureDefinition/familymemberhistory-type",
+  "version": "4.0.1",
+  "name": "type",
+  "title": "type",
+  "status": "draft",
+  "date": "2015-02-21",
+  "publisher": "Health Level Seven, Inc. - FHIR WG",
+  "contact": [{ "telecom": [{ "system": "url", "value": "HL7" }] }],
+  "description": "Purpose of the family member history or why it was created, such as when family member history is targeted for cardiovascular health, mental health, or genetic counseling.",
+  "fhirVersion": "4.0.1",
+  "mapping": [{ "identity": "rim", "uri": "http://hl7.org/v3", "name": "RIM Mapping" }],
+  "kind": "complex-type",
+  "abstract": false,
+  "context": [{ "type": "element", "expression": "FamilyMemberHistory" }],
+  "type": "Extension",
+  "baseDefinition": "http://hl7.org/fhir/StructureDefinition/Extension",
+  "derivation": "constraint",
+  "snapshot": {
+    "element": [
+      {
+        "id": "Extension",
+        "path": "Extension",
+        "short": "Purpose of the family member history",
+        "definition": "Purpose of the family member history or why it was created, such as when family member history is targeted for cardiovascular health, mental health, or genetic counseling.",
+        "comment": "List.code can be used to capture the type across a list of family members' histories.",
+        "min": 0,
+        "max": "1",
+        "base": { "path": "Extension", "min": 0, "max": "*" },
+        "condition": ["ele-1"],
+        "constraint": [
+          {
+            "key": "ele-1",
+            "severity": "error",
+            "human": "All FHIR elements must have a @value or children",
+            "expression": "hasValue() or (children().count() > id.count())",
+            "xpath": "@value|f:*|h:div",
+            "source": "http://hl7.org/fhir/StructureDefinition/Element"
+          },
+          {
+            "key": "ext-1",
+            "severity": "error",
+            "human": "Must have either extensions or value[x], not both",
+            "expression": "extension.exists() != value.exists()",
+            "xpath": "exists(f:extension)!=exists(f:*[starts-with(local-name(.), 'value')])",
+            "source": "http://hl7.org/fhir/StructureDefinition/Extension"
+          }
+        ],
+        "isModifier": false
+      },
+      {
+        "id": "Extension.id",
+        "path": "Extension.id",
+        "representation": ["xmlAttr"],
+        "short": "Unique id for inter-element referencing",
+        "definition": "Unique id for the element within a resource (for internal references). This may be any string value that does not contain spaces.",
+        "min": 0,
+        "max": "1",
+        "base": { "path": "Element.id", "min": 0, "max": "1" },
+        "type": [
+          {
+            "extension": [
+              {
+                "url": "http://hl7.org/fhir/StructureDefinition/structuredefinition-fhir-type",
+                "valueUrl": "string"
+              }
+            ],
+            "code": "http://hl7.org/fhirpath/System.String"
+          }
+        ],
+        "isModifier": false,
+        "isSummary": false,
+        "mapping": [{ "identity": "rim", "map": "n/a" }]
+      },
+      {
+        "id": "Extension.extension",
+        "path": "Extension.extension",
+        "slicing": {
+          "discriminator": [{ "type": "value", "path": "url" }],
+          "description": "Extensions are always sliced by (at least) url",
+          "rules": "open"
+        },
+        "short": "Extension",
+        "definition": "An Extension",
+        "min": 0,
+        "max": "0",
+        "base": { "path": "Element.extension", "min": 0, "max": "*" },
+        "type": [{ "code": "Extension" }],
+        "constraint": [
+          {
+            "key": "ele-1",
+            "severity": "error",
+            "human": "All FHIR elements must have a @value or children",
+            "expression": "hasValue() or (children().count() > id.count())",
+            "xpath": "@value|f:*|h:div",
+            "source": "http://hl7.org/fhir/StructureDefinition/Element"
+          },
+          {
+            "key": "ext-1",
+            "severity": "error",
+            "human": "Must have either extensions or value[x], not both",
+            "expression": "extension.exists() != value.exists()",
+            "xpath": "exists(f:extension)!=exists(f:*[starts-with(local-name(.), \"value\")])",
+            "source": "http://hl7.org/fhir/StructureDefinition/Extension"
+          }
+        ],
+        "isModifier": false,
+        "isSummary": false
+      },
+      {
+        "id": "Extension.url",
+        "path": "Extension.url",
+        "representation": ["xmlAttr"],
+        "short": "identifies the meaning of the extension",
+        "definition": "Source of the definition for the extension code - a logical name or a URL.",
+        "comment": "The definition may point directly to a computable or human-readable definition of the extensibility codes, or it may be a logical URI as declared in some other specification. The definition SHALL be a URI for the Structure Definition defining the extension.",
+        "min": 1,
+        "max": "1",
+        "base": { "path": "Extension.url", "min": 1, "max": "1" },
+        "type": [
+          {
+            "extension": [
+              {
+                "url": "http://hl7.org/fhir/StructureDefinition/structuredefinition-fhir-type",
+                "valueUrl": "uri"
+              }
+            ],
+            "code": "http://hl7.org/fhirpath/System.String"
+          }
+        ],
+        "fixedUri": "http://hl7.org/fhir/StructureDefinition/familymemberhistory-type",
+        "isModifier": false,
+        "isSummary": false,
+        "mapping": [{ "identity": "rim", "map": "N/A" }]
+      },
+      {
+        "id": "Extension.value[x]",
+        "path": "Extension.value[x]",
+        "short": "Value of extension",
+        "definition": "Value of extension - must be one of a constrained set of the data types (see [Extensibility](http://hl7.org/fhir/extensibility.html) for a list).",
+        "min": 1,
+        "max": "1",
+        "base": { "path": "Extension.value[x]", "min": 0, "max": "1" },
+        "type": [{ "code": "CodeableConcept" }],
+        "constraint": [
+          {
+            "key": "ele-1",
+            "severity": "error",
+            "human": "All FHIR elements must have a @value or children",
+            "expression": "hasValue() or (children().count() > id.count())",
+            "xpath": "@value|f:*|h:div",
+            "source": "http://hl7.org/fhir/StructureDefinition/Element"
+          }
+        ],
+        "isModifier": false,
+        "isSummary": false,
+        "mapping": [{ "identity": "rim", "map": "N/A" }]
+      }
+    ]
+  },
+  "differential": {
+    "element": [
+      {
+        "id": "Extension",
+        "path": "Extension",
+        "short": "Purpose of the family member history",
+        "definition": "Purpose of the family member history or why it was created, such as when family member history is targeted for cardiovascular health, mental health, or genetic counseling.",
+        "comment": "List.code can be used to capture the type across a list of family members' histories.",
+        "min": 0,
+        "max": "1"
+      },
+      { "id": "Extension.extension", "path": "Extension.extension", "max": "0" },
+      {
+        "id": "Extension.url",
+        "path": "Extension.url",
+        "fixedUri": "http://hl7.org/fhir/StructureDefinition/familymemberhistory-type"
+      },
+      {
+        "id": "Extension.value[x]",
+        "path": "Extension.value[x]",
+        "min": 1,
+        "type": [{ "code": "CodeableConcept" }]
+      }
+    ]
+  }
+}