From 86cc90d3e85068f3c23e581b5fb3ae54997f386c Mon Sep 17 00:00:00 2001 From: Mint Thompson Date: Tue, 28 Jun 2022 12:05:59 -0400 Subject: [PATCH] Check complex type values when validating assignments (#1108) When validating the a complex type value used in an AssignmentRule, the value may be an InstanceDefinition. Do not assign the value if it would conflict with existing values within the complex type. Assignment of complex values is all-or-nothing: if a conflict exists on part of the complex value, no parts of the complex value are assigned. An error is reported on the line where the Instance is assigned, and the error message refers to the specific value within the Instance that has a conflict. --- src/fhirtypes/ElementDefinition.ts | 37 +++++++- src/fhirtypes/StructureDefinition.ts | 9 +- test/export/InstanceExporter.test.ts | 123 +++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 7 deletions(-) diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts index 8fd781b4e..ffc0f1e86 100644 --- a/src/fhirtypes/ElementDefinition.ts +++ b/src/fhirtypes/ElementDefinition.ts @@ -1642,7 +1642,8 @@ export class ElementDefinition { stringVal, value.toJSON(), exactly, - value._instanceMeta.sdType ?? value.resourceType + value._instanceMeta.sdType ?? value.resourceType, + fisher ); break; default: @@ -1685,8 +1686,15 @@ export class ElementDefinition { * @throws {ValueAlreadyAssignedError} when the currentElementValue exists and is different than the new value * @throws {MismatchedTypeError} when the value does not match the type of the ElementDefinition */ - private assignFHIRValue(fshValue: string, fhirValue: any, exactly: boolean, type: string) { - if (this.type[0].code !== type) { + private assignFHIRValue( + fshValue: string, + fhirValue: any, + exactly: boolean, + type: string, + fisher?: Fishable + ) { + const lineage = fisher ? this.getTypeLineage(type, fisher).map(meta => meta.sdType) : []; + if (!this.type.some(t => t.code === type || lineage.includes(t.code))) { throw new MismatchedTypeError(type, fshValue, this.type[0].code); } @@ -1987,6 +1995,29 @@ export class ElementDefinition { metadata => metadata.sdType ); if (this.type?.some(t => lineage.includes(t.code))) { + // capture original value to restore after assignment + const originalKey = Object.keys(this).find( + k => k.startsWith('pattern') || k.startsWith('fixed') + ) as keyof ElementDefinition; + const originalValue = this[originalKey]; + // try assigning it to test for value conflicts + const stringVal = JSON.stringify(value); + this.assignFHIRValue( + stringVal, + value.toJSON(), + true, + value._instanceMeta.sdType ?? value.resourceType, + fisher + ); + // if the assignment is successful, undo it and return the value + const key = Object.keys(this).find( + k => k.startsWith('pattern') || k.startsWith('fixed') + ) as keyof ElementDefinition; + if (key != null) { + delete this[key]; + // @ts-ignore + this[originalKey] = originalValue; + } return value; } else { // In this case neither the type of the inline instance nor the type of any of its parents matches the diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts index 2cd6efaab..bba1bade0 100644 --- a/src/fhirtypes/StructureDefinition.ts +++ b/src/fhirtypes/StructureDefinition.ts @@ -676,16 +676,17 @@ export class StructureDefinition { previousElement = currentElement; } - const originalKey = Object.keys(currentElement).find( - k => k.startsWith('pattern') || k.startsWith('fixed') - ) as keyof ElementDefinition; - const originalValue = currentElement[originalKey]; let assignedValue; // Assigned resources cannot be assigned by pattern[x]/fixed[x], so we must set assignedValue directly if (value instanceof InstanceDefinition) { + // checkAssignInlineInstance will throw if it fails assignedValue = currentElement.checkAssignInlineInstance(value, fisher).toJSON(); } else { + const originalKey = Object.keys(currentElement).find( + k => k.startsWith('pattern') || k.startsWith('fixed') + ) as keyof ElementDefinition; + const originalValue = currentElement[originalKey]; // assignValue will throw if it fails, but skip the check if value is null if (value != null) { // exactly must be true so that we always test assigning with the more strict fixed[x] approach diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts index c0600337a..8c1d7ac58 100644 --- a/test/export/InstanceExporter.test.ts +++ b/test/export/InstanceExporter.test.ts @@ -4270,6 +4270,14 @@ describe('InstanceExporter', () => { // * valueString = "Some Observation" doc.instances.set(inlineObservation.name, inlineObservation); + const inlineOrganization = new Instance('MyInlineOrganization'); + inlineOrganization.instanceOf = 'Organization'; + const organizationName = new AssignmentRule('name'); + organizationName.value = 'Everyone'; + inlineOrganization.rules.push(organizationName); + // * name = "Everyone" + doc.instances.set(inlineOrganization.name, inlineOrganization); + const caretRule = new CaretValueRule('entry'); caretRule.caretPath = 'slicing.discriminator.type'; caretRule.value = new FshCode('value'); @@ -4384,6 +4392,21 @@ describe('InstanceExporter', () => { }); }); + it('should assign an inline resource that is not the first type to an instance element with a choice type', () => { + const bundleValRule = new AssignmentRule('entry[PatientOrOrganization].resource'); + bundleValRule.value = 'MyInlineOrganization'; + bundleValRule.isInstance = true; + // * entry[PatientOrOrganization].resource = MyInlineOrganization + bundleInstance.rules.push(bundleValRule); + + const exported = exportInstance(bundleInstance); + expect(exported.entry[0].resource).toEqual({ + resourceType: 'Organization', + id: 'MyInlineOrganization', + name: 'Everyone' + }); + }); + it('should assign an inline resource to an instance when the resource is not a profile and uses meta', () => { // This test reflects a real-world bug reported on Zulip: // https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/example.20FSH.20Bundle.20transaction.20with.20Create.20entries @@ -4715,6 +4738,106 @@ describe('InstanceExporter', () => { ]); }); + it('should assign an instance that matches existing values', () => { + // Profile: TestPatient + // Parent: Patient + // * name 1..1 + // * name.family = "Goodweather" + // * name.family 1..1 + // * name.text = "Regular text" + const nameCard = new CardRule('name'); + nameCard.min = 1; + nameCard.max = '1'; + const familyAssignment = new AssignmentRule('name.family'); + familyAssignment.value = 'Goodweather'; + const familyCard = new CardRule('name.family'); + familyCard.min = 1; + familyCard.max = '1'; + const textAssignment = new AssignmentRule('name.text'); + textAssignment.value = 'Regular text'; + patient.rules.push(nameCard, familyAssignment, familyCard, textAssignment); + // Instance: SameName + // InstanceOf: HumanName + // Usage: #inline + // * text = "Regular text" + // * use = #official + const sameName = new Instance('SameName'); + sameName.instanceOf = 'HumanName'; + sameName.usage = 'Inline'; + const sameText = new AssignmentRule('text'); + sameText.value = 'Regular text'; + const sameUse = new AssignmentRule('use'); + sameUse.value = new FshCode('official'); + sameName.rules.push(sameText, sameUse); + doc.instances.set(sameName.name, sameName); + // Instance: Bar + // InstanceOf: TestPatient + // * name = SameName + const nameAssignment = new AssignmentRule('name') + .withFile('Bar.fsh') + .withLocation([3, 3, 3, 25]); + nameAssignment.value = 'SameName'; + nameAssignment.isInstance = true; + patientInstance.rules.push(nameAssignment); + const exported = exportInstance(patientInstance); + expect(exported.name[0].family).toBe('Goodweather'); + expect(exported.name[0].text).toBe('Regular text'); + expect(exported.name[0].use).toBe('official'); + }); + + it('should log an error when assigning an instance that would overwrite an existing value', () => { + // Profile: TestPatient + // Parent: Patient + // * name 1..1 + // * name.family = "Goodweather" + // * name.family 1..1 + // * name.text = "Regular text" + const nameCard = new CardRule('name'); + nameCard.min = 1; + nameCard.max = '1'; + const familyAssignment = new AssignmentRule('name.family'); + familyAssignment.value = 'Goodweather'; + const familyCard = new CardRule('name.family'); + familyCard.min = 1; + familyCard.max = '1'; + const textAssignment = new AssignmentRule('name.text'); + textAssignment.value = 'Regular text'; + patient.rules.push(nameCard, familyAssignment, familyCard, textAssignment); + // Instance: DifferentName + // InstanceOf: HumanName + // Usage: #inline + // * text = "Different text" + // * use = #official + const differentName = new Instance('DifferentName'); + differentName.instanceOf = 'HumanName'; + differentName.usage = 'Inline'; + const differentText = new AssignmentRule('text'); + differentText.value = 'Different text'; + const differentUse = new AssignmentRule('use'); + differentUse.value = new FshCode('official'); + differentName.rules.push(differentText, differentUse); + doc.instances.set(differentName.name, differentName); + // Instance: Bar + // InstanceOf: TestPatient + // * name = DifferentName + const nameAssignment = new AssignmentRule('name') + .withFile('Bar.fsh') + .withLocation([3, 3, 3, 25]); + nameAssignment.value = 'DifferentName'; + nameAssignment.isInstance = true; + patientInstance.rules.push(nameAssignment); + const exported = exportInstance(patientInstance); + // name.family has a minimum cardinality of 1, so it is set as an implied property + expect(exported.name[0].family).toBe('Goodweather'); + // text is not required, but the assigned Instance's text would violate the Profile, so it is not assigned + expect(exported.name[0].text).toBeUndefined(); + // since the Instance is not assigned, the use is also undefined + expect(exported.name[0].use).toBeUndefined(); + expect(loggerSpy.getLastMessage('error')).toMatch( + /Cannot assign Different text to this element.*File: Bar\.fsh.*Line: 3\D*/s + ); + }); + it('should assign an instance of a type to an instance and log a warning when the type is not inline', () => { const inlineCodeable = new Instance('MyCodeable') .withFile('Code.fsh')