Skip to content

Commit

Permalink
Check complex type values when validating assignments (#1108)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mint-thompson authored Jun 28, 2022
1 parent f17591f commit 86cc90d
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 7 deletions.
37 changes: 34 additions & 3 deletions src/fhirtypes/ElementDefinition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1642,7 +1642,8 @@ export class ElementDefinition {
stringVal,
value.toJSON(),
exactly,
value._instanceMeta.sdType ?? value.resourceType
value._instanceMeta.sdType ?? value.resourceType,
fisher
);
break;
default:
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions src/fhirtypes/StructureDefinition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
123 changes: 123 additions & 0 deletions test/export/InstanceExporter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit 86cc90d

Please sign in to comment.