Repository: FHIR/sushi (ts)

Issue:
Canonical() should result in #fragment when referring to a contained resource
If a resource contains another resource, then within the container resource, `Canonical(ContainedResource)` should result in a fragment like `#contained-resource-id` instead of the full Canonical URL of the resource.

Example: https://fshschool.org/FSHOnline/#/share/3tnpgrc

Zulip: https://chat.fhir.org/#narrow/stream/179252-IG-creation/topic/Canonical.20reference.20to.20Contained.20resource/near/391011891

Pull request:
Canonical to non-Instance contained resources create fragment references
Fixes #1367, addresses [CIMPL-1189](https://standardhealthrecord.atlassian.net/browse/CIMPL-1189)

This PR makes it so that `Canonical()` references that refer to a contained resource (not created via an inline instance) result in a fragment (`#id`) rather than the full canonical URL. I've had some back and forth on the implementation on this, so a few areas to consider when reviewing this:

- completeness: This approach only addresses the issue within Instances. With the current version of SUSHI, using `Canonical` with an inline instance creates a fragment as expected. This adds support for using either assignment rules or non-Instance FSH entity to add a contained resource to an Instance. However, this PR doesn't change anything for other entity types, so StructureDefinitions still are not able to contain a ValueSet entity directly or with assignment rules. See some examples here: https://fshschool.org/FSHOnline/#/share/47Q1KTc.
- order of rules matters: This approach means that the contained rule needs to come before the reference using `Canonical`. I talked to Chris about this, and we seemed comfortable with that assumption at the time, but anyone is free to change their mind.
- maintainability: One reason I went with this approach to make a PR for is that for the most part, the changes are isolated to a function that can check for a relevant contained resource. Right now, that is only applied within the InstanceExporter. This could fairly easily be added to CodeSystemExporter and ValueSetExporter. It will take more work to support the same functionality in StructureDefinitionExporter.
- performance: Is looping through the validated contained resource rules for every rule that uses a Canonical keyword reasonable? When running a full regression, no repos indicated significant changes in execution time. I tried to limit when we loop through rules (only when the Assignment Rule uses the Canonical keyword) and the number of rules we loop through (only rules on `contained`), but it does add a new loop in our already slow Instance Exporter, so it is worth thinking about.

I ran a full regression and there were only four repos that had changes, which all were expected changes, `DigitalSQR/smart-immunizations-mini#main`, `ritikarawlani/smart-outbreak-measles#main`, `steveleshan/Measles1232#main`, and `WorldHealthOrganization/smart-ot#main`.

For bonus points / inquiring minds, the other approach for this task that I've implemented is in `[fragments-in-canonicals-v2](https://github.com/FHIR/sushi/compare/fragments-in-canonicals-v2)`. The quick summary of that PR is that it covers both Instances and StructureDefinitions with contained resources that reference FSH entities, but it does _not_ support contained resources added via direct assignment rules at the various contained paths on Instances or StructureDefinitions. It also touches more code and changes some fairly basic functions, like `assignString` and `assignFHIRValue`, which makes the code a little harder to maintain and felt a little less clean than this approach. The outcome of the full regression is the same on this branch.

Patch:
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 12d14e9..37b4aee 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -1,6 +1,6 @@
 import { FSHTank } from '../import/FSHTank';
 import { StructureDefinition, InstanceDefinition, ElementDefinition, PathPart } from '../fhirtypes';
-import { Instance, SourceInfo } from '../fshtypes';
+import { FshCanonical, Instance, SourceInfo } from '../fshtypes';
 import {
   logger,
   Fishable,
@@ -22,7 +22,8 @@ import {
   isModifierExtension,
   createUsefulSlices,
   determineKnownSlices,
-  setImpliedPropertiesOnInstance
+  setImpliedPropertiesOnInstance,
+  getMatchingContainedReferenceId
 } from '../fhirtypes/common';
 import { InstanceOfNotDefinedError } from '../errors/InstanceOfNotDefinedError';
 import { AbstractInstanceOfError } from '../errors/AbstractInstanceOfError';
@@ -159,10 +160,23 @@ export class InstanceExporter implements Fishable {
       string,
       { pathParts: PathPart[]; assignedValue: any; sourceInfo: SourceInfo }
     > = new Map();
+    // Keep track specifically of the rules on contained (path could be contained[index], contained.some-path, or contained)
+    const containedRules: { pathParts: PathPart[]; assignedValue: any }[] = [];
     rules.forEach(rule => {
       const inlineResourceTypes: string[] = [];
       // define function that will be re-used in attempting to assign a value or inline instance
       const doRuleValidation = (value: AssignmentValueType) => {
+        // Before validating the rule, check if the Canonical keyword was used to reference a contained value set
+        if (value instanceof FshCanonical) {
+          const entityName = value.entityName;
+          const matchingContainedReferenceId = getMatchingContainedReferenceId(
+            entityName,
+            containedRules
+          );
+          if (matchingContainedReferenceId) {
+            value = `#${matchingContainedReferenceId}`;
+          }
+        }
         const validatedRule = instanceOfStructureDefinition.validateValueAtPath(
           rule.path,
           value,
@@ -187,6 +201,11 @@ export class InstanceExporter implements Fishable {
             assignedValue: validatedRule.assignedValue,
             sourceInfo: rule.sourceInfo
           });
+          // Check if the rule we just validated was at a valid contained path to keep track for resolving Canonical references
+          // Only check if the rule was a directly contained resources (aka 'contained' or 'contained.name/url/id', optionally with slice names or indices)
+          if (/^contained(\[[^\]]+\])*(\.url|\.name|\.id)?$/.test(rule.path)) {
+            containedRules.push(validatedRule);
+          }
         }
       };
 
diff --git a/src/fhirtypes/common.ts b/src/fhirtypes/common.ts
index d6ce769..56a929b 100644
--- a/src/fhirtypes/common.ts
+++ b/src/fhirtypes/common.ts
@@ -1155,8 +1155,10 @@ export function cleanResource(
 
   // Update references to any contained resources to be #id instead of resourceType/id
   resourceDef.contained?.forEach((containedResource: any) => {
-    const referenceString = `${containedResource.resourceType}/${containedResource.id}`;
-    const referenceUrl = containedResource.url;
+    const referenceString = containedResource
+      ? `${containedResource.resourceType}/${containedResource.id}`
+      : null;
+    const referenceUrl = containedResource?.url;
     replaceField(
       resourceDef,
       (o, p) =>
@@ -1552,3 +1554,50 @@ export function findImposeProfiles(sd: any): string[] | undefined {
     return imposeProfiles;
   }
 }
+
+// Used to check if the entity used in Canonical() references a contained resource.
+// Checks a list of validated rules at any contained path for a matching value from the
+// Canonical() keyword and returns the matching resource's id.
+export function getMatchingContainedReferenceId(
+  value: string,
+  containedResources: { pathParts: PathPart[]; assignedValue: any }[]
+) {
+  const matchingContainedResource = containedResources.find(
+    r =>
+      r.assignedValue?.url === value ||
+      r.assignedValue?.name === value ||
+      r.assignedValue?.id === value
+  );
+  if (matchingContainedResource != null) {
+    return matchingContainedResource.assignedValue.id;
+  }
+
+  const matchingContainedResourceId = containedResources.find(
+    r => r.pathParts.slice(-1)[0].base === 'id' && r.assignedValue === value
+  );
+  const matchingContainedResourceName = containedResources.find(
+    r => r.pathParts.slice(-1)[0].base === 'name' && r.assignedValue === value
+  );
+  const matchingContainedResourceUrl = containedResources.find(
+    r => r.pathParts.slice(-1)[0].base === 'url' && r.assignedValue === value
+  );
+
+  if (
+    matchingContainedResourceId ||
+    matchingContainedResourceName ||
+    matchingContainedResourceUrl
+  ) {
+    const pathParts =
+      matchingContainedResourceId?.pathParts ??
+      matchingContainedResourceName?.pathParts ??
+      matchingContainedResourceUrl?.pathParts;
+    const containedResourceId = containedResources.find(
+      r =>
+        r.pathParts.slice(-1)[0].base === 'id' &&
+        isEqual(r.pathParts.slice(0, -1), pathParts.slice(0, -1))
+    );
+    if (containedResourceId) {
+      return containedResourceId.assignedValue;
+    }
+  }
+}


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index ae7ef60..42ab77f 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -6458,6 +6458,318 @@ describe('InstanceExporter', () => {
       expect(exported.instantiatesCanonical).toEqual(['http://example.org/PlanDefition/1']); // instantiatesCanonical is set
     });
 
+    it('should assign a Canonical as a #id fragment when referring to a contained resource created as a ValueSet entity', () => {
+      // ValueSet: ContainedValueSet
+      // Id: contained-value-set
+      const valueSet = new FshValueSet('ContainedValueSet');
+      valueSet.id = 'contained-value-set';
+      doc.valueSets.set(valueSet.name, valueSet);
+      // ValueSet: SecondContainedValueSet
+      // Id: second-contained-value-set
+      const valueSet2 = new FshValueSet('SecondContainedValueSet');
+      valueSet2.id = 'second-contained-value-set';
+      doc.valueSets.set(valueSet2.name, valueSet2);
+
+      // Instance: MyQuestionnaire
+      // InstanceOf: Questionnaire
+      const questionnaire = new Instance('MyQuestionnaire');
+      questionnaire.instanceOf = 'Questionnaire';
+      // * status = #active
+      const statusRule = new AssignmentRule('status');
+      statusRule.value = new FshCode('active');
+      // * contained = ContainedValueSet
+      const containedRule = new AssignmentRule('contained');
+      containedRule.value = 'ContainedValueSet';
+      containedRule.isInstance = true;
+      // * contained[1] = SecondContainedValueSet
+      const containedRule2 = new AssignmentRule('contained[1]');
+      containedRule2.value = 'SecondContainedValueSet';
+      containedRule2.isInstance = true;
+      // * item[0].linkId = "abc"
+      const itemLinkId = new AssignmentRule('item[0].linkId');
+      itemLinkId.value = 'abc';
+      // * item[=].type = #choice
+      const itemType = new AssignmentRule('item[=].type');
+      itemType.value = new FshCode('choice');
+      // * item[=].answerValueSet = Canonical(ContainedValueSet)
+      const itemAnswerValueSet = new AssignmentRule('item[=].answerValueSet');
+      itemAnswerValueSet.value = new FshCanonical('ContainedValueSet');
+      // * item[+].linkId = "xyz"
+      const itemLinkId2 = new AssignmentRule('item[+].linkId');
+      itemLinkId2.value = 'xyz';
+      // * item[=].type = #choice
+      const itemType2 = new AssignmentRule('item[=].type');
+      itemType2.value = new FshCode('choice');
+      // * item[=].answerValueSet = Canonical(SecondContainedValueSet)
+      const itemAnswerValueSet2 = new AssignmentRule('item[=].answerValueSet');
+      itemAnswerValueSet2.value = new FshCanonical('SecondContainedValueSet');
+      questionnaire.rules.push(
+        statusRule,
+        containedRule,
+        containedRule2,
+        itemLinkId,
+        itemType,
+        itemAnswerValueSet,
+        itemLinkId2,
+        itemType2,
+        itemAnswerValueSet2
+      );
+      doc.instances.set(questionnaire.name, questionnaire);
+
+      const exported = exportInstance(questionnaire);
+      expect(exported.contained).toHaveLength(2);
+      expect(exported.contained[0].id).toEqual('contained-value-set');
+      expect(exported.contained[1].id).toEqual('second-contained-value-set');
+      expect(exported.item).toHaveLength(2);
+      expect(exported.item[0].answerValueSet).toEqual('#contained-value-set');
+      expect(exported.item[1].answerValueSet).toEqual('#second-contained-value-set');
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+      expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+    });
+
+    it('should assign a Canonical as a #id fragment when referring to a contained resource created directly on the instance', () => {
+      // Instance: MyQuestionnaire
+      // InstanceOf: Questionnaire
+      const questionnaire = new Instance('MyQuestionnaire');
+      questionnaire.instanceOf = 'Questionnaire';
+      // * status = #active
+      const statusRule = new AssignmentRule('status');
+      statusRule.value = new FshCode('active');
+      // * contained[+].resourceType = "ValueSet"
+      const containedResourceType = new AssignmentRule('contained[+].resourceType');
+      containedResourceType.value = 'ValueSet';
+      // * contained[=].id = "directly-contained-value-set"
+      const containedId = new AssignmentRule('contained[=].id');
+      containedId.value = 'directly-contained-value-set';
+      // * contained[=].name = "DirectlyContainedValueSet"
+      const containedName = new AssignmentRule('contained[=].name');
+      containedName.value = 'DirectlyContainedValueSet';
+      // * contained[=].url = "http://hl7.org/fhir/us/minimal/ValueSet/DirectlyContainedValueSet"
+      const containedUrl = new AssignmentRule('contained[=].url');
+      containedUrl.value = 'http://hl7.org/fhir/us/minimal/ValueSet/DirectlyContainedValueSet';
+      // * contained[+].resourceType = "ValueSet"
+      const containedResourceType2 = new AssignmentRule('contained[+].resourceType');
+      containedResourceType2.value = 'ValueSet';
+      // * contained[=].id = "directly-contained-value-set-2"
+      const containedId2 = new AssignmentRule('contained[=].id');
+      containedId2.value = 'directly-contained-value-set-2';
+      // * contained[=].name = "DirectlyContainedValueSet2"
+      const containedName2 = new AssignmentRule('contained[=].name');
+      containedName2.value = 'DirectlyContainedValueSet2';
+      // * contained[=].url = "http://hl7.org/fhir/us/minimal/ValueSet/DirectlyContainedValueSet2"
+      const containedUrl2 = new AssignmentRule('contained[=].url');
+      containedUrl2.value = 'http://hl7.org/fhir/us/minimal/ValueSet/DirectlyContainedValueSet2';
+      // * item[0].linkId = "abc"
+      const itemLinkId = new AssignmentRule('item[0].linkId');
+      itemLinkId.value = 'abc';
+      // * item[=].type = #choice
+      const itemType = new AssignmentRule('item[=].type');
+      itemType.value = new FshCode('choice');
+      // * item[=].answerValueSet = Canonical(DirectlyContainedValueSet2)
+      const itemAnswerValueSet = new AssignmentRule('item[=].answerValueSet');
+      itemAnswerValueSet.value = new FshCanonical('DirectlyContainedValueSet2');
+      questionnaire.rules.push(
+        statusRule,
+        containedResourceType,
+        containedId,
+        containedName,
+        containedUrl,
+        containedResourceType2,
+        containedId2,
+        containedName2,
+        containedUrl2,
+        itemLinkId,
+        itemType,
+        itemAnswerValueSet
+      );
+      doc.instances.set(questionnaire.name, questionnaire);
+
+      const exported = exportInstance(questionnaire);
+      expect(exported.contained).toHaveLength(2);
+      expect(exported.contained[0].id).toEqual('directly-contained-value-set');
+      expect(exported.contained[1].id).toEqual('directly-contained-value-set-2');
+      expect(exported.item).toHaveLength(1);
+      expect(exported.item[0].answerValueSet).toEqual('#directly-contained-value-set-2');
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+      expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+    });
+
+    it('should assign a Canonical as a #id fragment when referring to a contained resource that was added by slice name, slice name with index, and double digit indices', () => {
+      // Profile: QuestionnaireProfile
+      // Parent:  Questionnaire
+      // * contained ^slicing.discriminator.type = #pattern
+      // * contained ^slicing.discriminator.path = "$this"
+      // * contained ^slicing.rules = #open
+      // * contained contains myvs 0..*
+      const questionnaireProfile = new Profile('QuestionnaireProfile');
+      questionnaireProfile.parent = 'Questionnaire';
+      const slicingType = new CaretValueRule('contained');
+      slicingType.caretPath = 'slicing.discriminator.type';
+      slicingType.value = new FshCode('pattern');
+      const slicingPath = new CaretValueRule('contained');
+      slicingPath.caretPath = 'slicing.discriminator.path';
+      slicingPath.value = 'this';
+      const slicingRules = new CaretValueRule('contained');
+      slicingRules.caretPath = 'slicing.rules';
+      slicingRules.value = new FshCode('open');
+      const containedContains = new ContainsRule('contained');
+      containedContains.items.push({ name: 'myvs' });
+      const myvsCard = new CardRule('contained[myvs]');
+      myvsCard.min = 0;
+      myvsCard.max = '*';
+      questionnaireProfile.rules.push(
+        slicingType,
+        slicingPath,
+        slicingRules,
+        containedContains,
+        myvsCard
+      );
+      doc.profiles.set(questionnaireProfile.name, questionnaireProfile);
+
+      // ValueSet: ContainedValueSet
+      // Id: contained-value-set
+      const valueSet = new FshValueSet('ContainedValueSet');
+      valueSet.id = 'contained-value-set';
+      doc.valueSets.set(valueSet.name, valueSet);
+      // ValueSet: SecondContainedValueSet
+      // Id: second-contained-value-set
+      const valueSet2 = new FshValueSet('SecondContainedValueSet');
+      valueSet2.id = 'second-contained-value-set';
+      doc.valueSets.set(valueSet2.name, valueSet2);
+      // ValueSet: ThirdContainedValueSet
+      // Id: third-contained-value-set
+      const valueSet3 = new FshValueSet('ThirdContainedValueSet');
+      valueSet3.id = 'third-contained-value-set';
+      doc.valueSets.set(valueSet3.name, valueSet3);
+
+      // Instance: MyQuestionnaire
+      // InstanceOf: QuestionnaireProfile
+      const questionnaire = new Instance('MyQuestionnaire');
+      questionnaire.instanceOf = 'QuestionnaireProfile';
+      // * status = #active
+      const statusRule = new AssignmentRule('status');
+      statusRule.value = new FshCode('active');
+      // * contained[myvs] = ContainedValueSet
+      const containedRule = new AssignmentRule('contained[myvs]');
+      containedRule.value = 'ContainedValueSet';
+      containedRule.isInstance = true;
+      // * contained[myvs][1] = SecondContainedValueSet
+      const containedRule2 = new AssignmentRule('contained[myvs][1]');
+      containedRule2.value = 'SecondContainedValueSet';
+      containedRule2.isInstance = true;
+      // * contained[10] = ThirdContainedValueSet
+      const containedRule3 = new AssignmentRule('contained[10]');
+      containedRule3.value = 'ThirdContainedValueSet';
+      containedRule3.isInstance = true;
+      // * item[0].linkId = "abc"
+      const itemLinkId = new AssignmentRule('item[0].linkId');
+      itemLinkId.value = 'abc';
+      // * item[=].type = #choice
+      const itemType = new AssignmentRule('item[=].type');
+      itemType.value = new FshCode('choice');
+      // * item[=].answerValueSet = Canonical(ContainedValueSet)
+      const itemAnswerValueSet = new AssignmentRule('item[=].answerValueSet');
+      itemAnswerValueSet.value = new FshCanonical('ContainedValueSet');
+      // * item[+].linkId = "xyz"
+      const itemLinkId2 = new AssignmentRule('item[+].linkId');
+      itemLinkId2.value = 'xyz';
+      // * item[=].type = #choice
+      const itemType2 = new AssignmentRule('item[=].type');
+      itemType2.value = new FshCode('choice');
+      // * item[=].answerValueSet = Canonical(SecondContainedValueSet)
+      const itemAnswerValueSet2 = new AssignmentRule('item[=].answerValueSet');
+      itemAnswerValueSet2.value = new FshCanonical('SecondContainedValueSet');
+      // * item[+].linkId = "mno"
+      const itemLinkId3 = new AssignmentRule('item[+].linkId');
+      itemLinkId3.value = 'mno';
+      // * item[=].type = #choice
+      const itemType3 = new AssignmentRule('item[=].type');
+      itemType3.value = new FshCode('choice');
+      // * item[=].answerValueSet = Canonical(ThirdContainedValueSet)
+      const itemAnswerValueSet3 = new AssignmentRule('item[=].answerValueSet');
+      itemAnswerValueSet3.value = new FshCanonical('ThirdContainedValueSet');
+      questionnaire.rules.push(
+        statusRule,
+        containedRule,
+        containedRule2,
+        containedRule3,
+        itemLinkId,
+        itemType,
+        itemAnswerValueSet,
+        itemLinkId2,
+        itemType2,
+        itemAnswerValueSet2,
+        itemLinkId3,
+        itemType3,
+        itemAnswerValueSet3
+      );
+      doc.instances.set(questionnaire.name, questionnaire);
+
+      const exported = exportInstance(questionnaire);
+      expect(exported.contained).toHaveLength(11);
+      expect(exported.contained[0].id).toEqual('contained-value-set');
+      expect(exported.contained[1].id).toEqual('second-contained-value-set');
+      expect(exported.contained[2]).toBeNull(); // 2 - 9 are null
+      expect(exported.contained[10].id).toEqual('third-contained-value-set');
+      expect(exported.item).toHaveLength(3);
+      expect(exported.item[0].answerValueSet).toEqual('#contained-value-set');
+      expect(exported.item[1].answerValueSet).toEqual('#second-contained-value-set');
+      expect(exported.item[2].answerValueSet).toEqual('#third-contained-value-set');
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+      expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+    });
+
+    it('should assign a Canonical as a full url (not #id) when referring to a resource that is not directly on the contained array', () => {
+      // ValueSet: ContainedValueSet
+      // Id: contained-value-set
+      const valueSet = new FshValueSet('ContainedValueSet');
+      valueSet.id = 'contained-value-set';
+      doc.valueSets.set(valueSet.name, valueSet);
+
+      // Instance: MyQuestionnaire
+      // InstanceOf: Questionnaire
+      const questionnaire = new Instance('MyQuestionnaire');
+      questionnaire.instanceOf = 'Questionnaire';
+      // * status = #active
+      const statusRule = new AssignmentRule('status');
+      statusRule.value = new FshCode('active');
+      // * contained[0].resourceType = "Bundle"
+      const containedResourceTypeRule = new AssignmentRule('contained[0].resourceType');
+      containedResourceTypeRule.value = 'Bundle';
+      // * contained[0].entry[0].resource = ContainedValueSet
+      const containedEntryResourceRule = new AssignmentRule('contained[0].entry[0].resource');
+      containedEntryResourceRule.value = 'ContainedValueSet';
+      containedEntryResourceRule.isInstance = true;
+      // * item[0].linkId = "abc"
+      const itemLinkId = new AssignmentRule('item[0].linkId');
+      itemLinkId.value = 'abc';
+      // * item[=].type = #choice
+      const itemType = new AssignmentRule('item[=].type');
+      itemType.value = new FshCode('choice');
+      // * item[=].answerValueSet = Canonical(ContainedValueSet)
+      const itemAnswerValueSet = new AssignmentRule('item[=].answerValueSet');
+      itemAnswerValueSet.value = new FshCanonical('ContainedValueSet');
+      questionnaire.rules.push(
+        statusRule,
+        containedResourceTypeRule,
+        containedEntryResourceRule,
+        itemLinkId,
+        itemType,
+        itemAnswerValueSet
+      );
+      doc.instances.set(questionnaire.name, questionnaire);
+
+      const exported = exportInstance(questionnaire);
+      expect(exported.contained).toHaveLength(1);
+      expect(exported.contained[0].entry[0].resource.id).toEqual('contained-value-set');
+      expect(exported.item).toHaveLength(1);
+      expect(exported.item[0].answerValueSet).toEqual(
+        'http://hl7.org/fhir/us/minimal/ValueSet/contained-value-set'
+      );
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+      expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+    });
+
     it('should log an error when an invalid canonical is assigned', () => {
       const assignedRefRule = new AssignmentRule('instantiatesCanonical');
       const vsInstance = new Instance('TestVS');