Skip to content

Commit

Permalink
Fixes #3753 - root constraint validation (#3779)
Browse files Browse the repository at this point in the history
* Fixes #3753 - root constraint validation

* Fixed constraint bugs in core

* Fixed server tests

* More structure definition fixes

* Fixed structure definition

* Temp skip org-1 constraint
  • Loading branch information
codyebberson committed Jan 21, 2024
1 parent 1365831 commit d341cd1
Show file tree
Hide file tree
Showing 11 changed files with 219 additions and 48 deletions.
6 changes: 6 additions & 0 deletions packages/core/src/fhirpath/functions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ describe('FHIRPath functions', () => {
expect(functions.empty(context, [TYPED_1, TYPED_2])).toEqual([TYPED_FALSE]);
});

test('hasValue', () => {
expect(functions.hasValue(context, [])).toEqual([TYPED_FALSE]);
expect(functions.hasValue(context, [TYPED_1])).toEqual([TYPED_TRUE]);
expect(functions.hasValue(context, [TYPED_1, TYPED_2])).toEqual([TYPED_TRUE]);
});

test('exists', () => {
expect(functions.exists(context, [])).toEqual([TYPED_FALSE]);
expect(functions.exists(context, [TYPED_1])).toEqual([TYPED_TRUE]);
Expand Down
18 changes: 14 additions & 4 deletions packages/core/src/fhirpath/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ export const functions: Record<string, FhirPathFunction> = {
return booleanToTypedValue(input.length === 0);
},

/**
* Returns true if the input collection is not empty ({ }) and false otherwise.
*
* @param _context - The evaluation context.
* @param input - The input collection.
* @returns True if the input collection is not empty ({ }) and false otherwise.
*/
hasValue: (_context: AtomContext, input: TypedValue[]): TypedValue[] => {
return booleanToTypedValue(input.length !== 0);
},

/**
* Returns true if the collection has unknown elements, and false otherwise.
* This is the opposite of empty(), and as such is a shorthand for empty().not().
Expand Down Expand Up @@ -1493,13 +1504,12 @@ export const functions: Record<string, FhirPathFunction> = {
* function unchanged.
*
* See: https://hl7.org/fhirpath/#tracename-string-projection-expression-collection
* @param context - The evaluation context.
* @param _context - The evaluation context.
* @param input - The input collection.
* @param nameAtom - The log name.
* @param _nameAtom - The log name.
* @returns The input collection.
*/
trace: (context: AtomContext, input: TypedValue[], nameAtom: Atom): TypedValue[] => {
console.log('trace', input, nameAtom);
trace: (_context: AtomContext, input: TypedValue[], _nameAtom: Atom): TypedValue[] => {
return input;
},

Expand Down
62 changes: 61 additions & 1 deletion packages/core/src/fhirpath/parse.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { readJson } from '@medplum/definitions';
import { Bundle, BundleEntry, Observation, Patient, SearchParameter } from '@medplum/fhirtypes';
import { Bundle, BundleEntry, Encounter, Observation, Patient, SearchParameter } from '@medplum/fhirtypes';
import { PropertyType } from '../types';
import { indexStructureDefinitionBundle } from '../typeschema/types';
import { evalFhirPath, evalFhirPathTyped, parseFhirPath } from './parse';
Expand Down Expand Up @@ -601,4 +601,64 @@ describe('FHIRPath parser', () => {
expect(evalFhirPath(expr, { concept, system })).toEqual([true]);
expect(evalFhirPath(expr, { concept, filter, system })).toEqual([true]);
});

test('where and', () => {
const expr = "identifier.where(system='http://example.com' and value='123').exists()";

const e1: Encounter = {
resourceType: 'Encounter',
status: 'finished',
class: { code: 'foo' },
identifier: [{ system: 'http://example.com', value: '123' }],
};
expect(evalFhirPath(expr, e1)).toEqual([true]);

const e2: Encounter = {
resourceType: 'Encounter',
status: 'finished',
class: { code: 'foo' },
identifier: [{ system: 'http://example.com', value: '456' }],
};
expect(evalFhirPath(expr, e2)).toEqual([false]);
});

test('Bundle bdl-3', () => {
// entry.request mandatory for batch/transaction/history, otherwise prohibited
const expr =
"entry.all(request.exists() = (%resource.type = 'batch' or %resource.type = 'transaction' or %resource.type = 'history'))";

const b1: Bundle = {
resourceType: 'Bundle',
type: 'batch',
entry: [
{
request: {
method: 'POST',
url: 'Patient',
},
resource: {
resourceType: 'Patient',
id: '123',
},
},
],
};
const tb1 = toTypedValue(b1);
expect(evalFhirPathTyped(expr, [tb1], { '%resource': tb1 })).toEqual([toTypedValue(true)]);

const b2: Bundle = {
resourceType: 'Bundle',
type: 'collection',
entry: [
{
resource: {
resourceType: 'Patient',
id: '123',
},
},
],
};
const tb2 = toTypedValue(b2);
expect(evalFhirPathTyped(expr, [tb2], { '%resource': tb2 })).toEqual([toTypedValue(true)]);
});
});
2 changes: 1 addition & 1 deletion packages/core/src/typeschema/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ class StructureDefinitionParser {

private parseSliceStart(element: ElementDefinition): void {
if (!this.slicingContext) {
throw new Error('Invalid slice start before discriminator: ' + element.sliceName);
throw new Error(`Invalid slice start before discriminator: ${element.sliceName} (${element.id})`);
}
if (this.slicingContext.current) {
this.slicingContext.field.slices.push(this.slicingContext.current);
Expand Down
120 changes: 88 additions & 32 deletions packages/core/src/typeschema/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
CodeSystem,
Condition,
DiagnosticReport,
ElementDefinition,
Encounter,
Extension,
HumanName,
ImplementationGuide,
Expand All @@ -19,6 +21,7 @@ import {
QuestionnaireItem,
Resource,
StructureDefinition,
StructureDefinitionSnapshot,
SubstanceProtein,
ValueSet,
} from '@medplum/fhirtypes';
Expand All @@ -27,19 +30,27 @@ import { resolve } from 'path';
import { LOINC, RXNORM, SNOMED, UCUM } from '../constants';
import { ContentType } from '../contenttype';
import { OperationOutcomeError } from '../outcomes';
import { createReference } from '../utils';
import { createReference, deepClone } from '../utils';
import { indexStructureDefinitionBundle } from './types';
import { validateResource } from './validation';

describe('FHIR resource validation', () => {
let typesBundle: Bundle;
let resourcesBundle: Bundle;
let medplumBundle: Bundle;
let observationProfile: StructureDefinition;
let patientProfile: StructureDefinition;

beforeAll(() => {
console.log = jest.fn();
indexStructureDefinitionBundle(readJson('fhir/r4/profiles-types.json') as Bundle);
indexStructureDefinitionBundle(readJson('fhir/r4/profiles-resources.json') as Bundle);
indexStructureDefinitionBundle(readJson('fhir/r4/profiles-medplum.json') as Bundle);

typesBundle = readJson('fhir/r4/profiles-types.json') as Bundle;
resourcesBundle = readJson('fhir/r4/profiles-resources.json') as Bundle;
medplumBundle = readJson('fhir/r4/profiles-medplum.json') as Bundle;

indexStructureDefinitionBundle(typesBundle);
indexStructureDefinitionBundle(resourcesBundle);
indexStructureDefinitionBundle(medplumBundle);

observationProfile = JSON.parse(
readFileSync(resolve(__dirname, '__test__', 'us-core-blood-pressure.json'), 'utf8')
Expand Down Expand Up @@ -449,14 +460,9 @@ describe('FHIR resource validation', () => {
});

test('StructureDefinition', () => {
expect(() => {
const structureDefinition = readJson('fhir/r4/profiles-resources.json') as Bundle;
validateResource(structureDefinition);
}).not.toThrow();
expect(() => {
const structureDefinition = readJson('fhir/r4/profiles-medplum.json') as Bundle;
validateResource(structureDefinition);
}).not.toThrow();
expect(() => validateResource(typesBundle)).not.toThrow();
expect(() => validateResource(resourcesBundle)).not.toThrow();
expect(() => validateResource(medplumBundle)).not.toThrow();
});

test('Profile with restriction on base type field', () => {
Expand Down Expand Up @@ -741,14 +747,6 @@ describe('FHIR resource validation', () => {
};
expect(() => validateResource(observation, bodyWeightProfile as StructureDefinition)).not.toThrow();
});
});

describe('Legacy tests for parity checking', () => {
beforeAll(() => {
indexStructureDefinitionBundle(readJson('fhir/r4/profiles-types.json') as Bundle);
indexStructureDefinitionBundle(readJson('fhir/r4/profiles-resources.json') as Bundle);
indexStructureDefinitionBundle(readJson('fhir/r4/profiles-medplum.json') as Bundle);
});

test('validateResource', () => {
expect(() => validateResource(null as unknown as Resource)).toThrow();
Expand Down Expand Up @@ -1088,6 +1086,8 @@ describe('Legacy tests for parity checking', () => {
const appt: Appointment = {
resourceType: 'Appointment',
status: 'booked',
start: '2022-02-02T12:00:00Z',
end: '2022-02-02T12:30:00Z',
participant: [{ status: 'accepted', actor: patientReference }],
};

Expand Down Expand Up @@ -1121,6 +1121,8 @@ describe('Legacy tests for parity checking', () => {
const appt: Appointment = {
resourceType: 'Appointment',
status: 'booked',
start: '2022-02-02T12:00:00Z',
end: '2022-02-02T12:30:00Z',
participant: [{ status: 'accepted', actor: patientReference }],
};

Expand Down Expand Up @@ -1158,20 +1160,17 @@ describe('Legacy tests for parity checking', () => {
fail('Expected error');
} catch (err) {
const outcome = (err as OperationOutcomeError).outcome;
expect(outcome.issue).toHaveLength(1);
expect(outcome.issue).toHaveLength(2);

expect(outcome.issue?.[0]?.severity).toEqual('error');
expect(outcome.issue?.[0]?.details?.text).toEqual('Missing required property');
expect(outcome.issue?.[0]?.expression?.[0]).toEqual('Appointment.participant.status');
}
});
expect(outcome.issue?.[0]?.details?.text).toEqual(
'Constraint app-3 not met: Only proposed or cancelled appointments can be missing start/end dates'
);
expect(outcome.issue?.[0]?.expression?.[0]).toEqual('Appointment');

test('StructureDefinition', () => {
const structureDefinition = readJson('fhir/r4/profiles-resources.json') as Bundle;
try {
validateResource(structureDefinition);
} catch (err) {
const outcome = (err as OperationOutcomeError).outcome;
console.log(JSON.stringify(outcome, null, 2).substring(0, 1000));
expect(outcome.issue?.[1]?.severity).toEqual('error');
expect(outcome.issue?.[1]?.details?.text).toEqual('Missing required property');
expect(outcome.issue?.[1]?.expression?.[0]).toEqual('Appointment.participant.status');
}
});

Expand Down Expand Up @@ -1293,6 +1292,63 @@ describe('Legacy tests for parity checking', () => {

expect(() => validateResource(resource)).not.toThrow();
});

test('where identifier exists', () => {
const original = resourcesBundle.entry?.find((e) => e.resource?.id === 'Encounter')
?.resource as StructureDefinition;

expect(original).toBeDefined();

const profile = deepClone(original);

const rootElement = (profile.snapshot as StructureDefinitionSnapshot).element?.find(
(e) => e.id === 'Encounter'
) as ElementDefinition;
rootElement.constraint = [
{
key: 'where-identifier-exists',
expression: "identifier.where(system='http://example.com' and value='123').exists()",
severity: 'error',
human: 'Identifier must exist',
},
];

const identifierElement = (profile.snapshot as StructureDefinitionSnapshot).element?.find(
(e) => e.id === 'Encounter.identifier'
) as ElementDefinition;
identifierElement.min = 1;
identifierElement.constraint = [
{
key: 'where-identifier-exists',
expression: "where(system='http://example.com' and value='123').exists()",
severity: 'error',
human: 'Identifier must exist',
},
];

const e1: Encounter = {
resourceType: 'Encounter',
status: 'finished',
class: { code: 'foo' },
};
expect(() => validateResource(e1, profile)).toThrow();

const e2: Encounter = {
resourceType: 'Encounter',
status: 'finished',
class: { code: 'foo' },
identifier: [{ system: 'http://example.com', value: '123' }],
};
expect(() => validateResource(e2, profile)).not.toThrow();

const e3: Encounter = {
resourceType: 'Encounter',
status: 'finished',
class: { code: 'foo' },
identifier: [{ system: 'http://example.com', value: '456' }],
};
expect(() => validateResource(e3, profile)).toThrow();
});
});

function fail(reason: string): never {
Expand Down
17 changes: 13 additions & 4 deletions packages/core/src/typeschema/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,12 @@ const validationRegexes: Record<string, RegExp> = {
/**
* List of constraint keys that aren't to be checked in an expression.
*/
const skippedConstraintKeys: Record<string, boolean> = { 'ele-1': true };
const skippedConstraintKeys: Record<string, boolean> = {
'ele-1': true,
'dom-3': true, // If the resource is contained in another resource, it SHALL be referred to from elsewhere in the resource (requries "descendants()")
'org-1': true, // The organization SHALL at least have a name or an identifier, and possibly more than one (back compat)
'sdf-19': true, // FHIR Specification models only use FHIR defined types
};

export function validateResource(resource: Resource, profile?: StructureDefinition): void {
new ResourceValidator(resource.resourceType, resource, profile).validate();
Expand All @@ -97,7 +102,7 @@ class ResourceValidator implements ResourceVisitor {
constructor(resourceType: string, rootResource: Resource, profile?: StructureDefinition) {
this.issues = [];
this.rootResource = rootResource;
this.currentResource = [];
this.currentResource = [rootResource];
if (!profile) {
this.schema = getDataType(resourceType);
} else {
Expand All @@ -111,6 +116,9 @@ class ResourceValidator implements ResourceVisitor {
throw new OperationOutcomeError(validationError('Missing resource type'));
}

// Check root constraints
this.constraintsCheck(toTypedValue(this.rootResource), this.schema, resourceType);

checkObjectForNull(this.rootResource as unknown as Record<string, unknown>, resourceType, this.issues);

crawlResource(this.rootResource, this, this.schema);
Expand Down Expand Up @@ -173,7 +181,7 @@ class ResourceValidator implements ResourceVisitor {
if (values.length < element.min || values.length > element.max) {
this.issues.push(
createStructureIssue(
path,
element.path,
`Invalid number of values: expected ${element.min}..${
Number.isFinite(element.max) ? element.max : '*'
}, but found ${values.length}`
Expand All @@ -195,6 +203,7 @@ class ResourceValidator implements ResourceVisitor {
sliceCounts[sliceName] += 1;
}
}

this.validateSlices(element.slicing?.slices, sliceCounts, path);
}
}
Expand Down Expand Up @@ -269,7 +278,7 @@ class ResourceValidator implements ResourceVisitor {
}
}

private constraintsCheck(value: TypedValue, field: InternalSchemaElement, path: string): void {
private constraintsCheck(value: TypedValue, field: InternalTypeSchema | InternalSchemaElement, path: string): void {
const constraints = field.constraints;
if (!constraints) {
return;
Expand Down
Loading

2 comments on commit d341cd1

@vercel
Copy link

@vercel vercel bot commented on d341cd1 Jan 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

medplum-storybook – ./

medplum-storybook-medplum.vercel.app
medplum-storybook-git-main-medplum.vercel.app
storybook.medplum.com

@vercel
Copy link

@vercel vercel bot commented on d341cd1 Jan 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

medplum-www – ./packages/docs

medplum-www-medplum.vercel.app
www.medplum.com
medplum-www-git-main-medplum.vercel.app
medplum-www.vercel.app
medplum.com

Please sign in to comment.