diff --git a/tests/tx/validate-deadline.test.js b/tests/tx/validate-deadline.test.js new file mode 100644 index 00000000..0f73c4b7 --- /dev/null +++ b/tests/tx/validate-deadline.test.js @@ -0,0 +1,80 @@ +/** + * checkConceptSet honours the operation's time limit while it looks for a listed code + * written in another form. + * + * When the code being validated is not one of an include's listed codes and the code system + * says the code has another form (ICD-11 short code vs uri), checkConceptSet asks, for every + * listed code, whether the two are the same concept (sameConceptDifferentForm): locate() calls + * per listed code. The loop had no deadline check, so with a slow code system it ran to the + * end, however long that took, before the next loop noticed the time limit had passed. + */ + +const { Languages } = require('../../library/languages'); +const { OperationContext } = require('../../tx/operation-context'); +const { Renderer } = require('../../tx/library/renderer'); +const { ValueSetChecker } = require('../../tx/workers/validate'); + +/** Burn CPU synchronously for ms milliseconds (the deadline is charged in compute time). */ +function spin(ms) { + const end = performance.now() + ms; + while (performance.now() < end) { /* burn */ } +} + +/** A code system whose codes have another form (as ICD-11's do), where every lookup costs 1 ms of compute. */ +function slowCodeSystem() { + const cs = { + locates: 0, + async locate(code) { + cs.locates++; + spin(1); + return { context: { code }, message: undefined }; + }, + async classificationCode() { return 'other-form'; }, + async code(context) { return context.code; }, + system: () => 'http://example.org/slow', + version: () => null + }; + return cs; +} + +function checkerFor(opContext) { + // only the parts of a ValueSetChecker that checkConceptSet uses on this path + const checker = Object.create(ValueSetChecker.prototype); + checker.worker = { + opContext, + renderer: new Renderer(null, null, null), + deadCheck: (place) => opContext.deadCheck(place) + }; + checker.valueSet = { vurl: 'http://example.org/vs' }; + checker.params = {}; + checker.indentCount = 0; + return checker; +} + +describe('checkConceptSet deadline', () => { + test('stops looking for another form of the code once the time limit has passed', async () => { + const LISTED = 3000; // ~3 s of lookups at 1 ms each + const opContext = new OperationContext(Languages.fromAcceptLanguage('en'), null, null, 0.2); // 200 ms + opContext.debugging = false; // a debugger attached to jest would otherwise disable the check + const cs = slowCodeSystem(); + const cset = { system: 'http://example.org/slow', concept: Array.from({ length: LISTED }, (_, i) => ({ code: 'c' + i })) }; + + const start = performance.now(); + let error = null; + try { + await checkerFor(opContext).checkConceptSet('Coding', 'in', cs, cset, 'not-listed', null, + { vurl: 'http://example.org/vs' }, null, {}, {}, {}, null, null, []); + } catch (e) { + error = e; + } + const took = performance.now() - start; + + expect(error).not.toBeNull(); + expect(error.message).toContain('Operation exceeded time limit'); + expect(error.message).toContain('checkConceptSet#3'); + // the check is sampled (every CHECK_FREQUENCY calls), so a few hundred lookups past + // the limit, not all 3,000 of them + expect(cs.locates).toBeLessThan(LISTED / 2); + expect(took).toBeLessThan(1500); + }); +}); diff --git a/tests/tx/validate-enumerated-miss.test.js b/tests/tx/validate-enumerated-miss.test.js new file mode 100644 index 00000000..1f49e44a --- /dev/null +++ b/tests/tx/validate-enumerated-miss.test.js @@ -0,0 +1,135 @@ +/** + * A code that is not in an enumerated include costs one lookup of the code, not one per listed code. + * + * Before checking whether a listed concept is the same concept written in another form (ICD-11's + * short code and entity id), the checker located the code once per listed concept, though the + * answer (does the code have another form at all?) does not depend on the listed concept. A miss + * against a 33,000-code VSAC list never finished. It is now asked once per include. + */ +const request = require('supertest'); +const { getTestApp, shutdownTestApp } = require('./setup'); +const { FhirCodeSystemProvider } = require('../../tx/cs/cs-cs'); +const { ValueSetChecker } = require('../../tx/workers/validate'); + +const CS_URL = 'http://example.org/fhir/CodeSystem/enumerated-miss-cs'; +const VS_URL = 'http://example.org/fhir/ValueSet/enumerated-miss-vs'; +const N = 300; +const codes = Array.from({ length: N }, (_, i) => 'c' + i); +const CS = { + resourceType: 'CodeSystem', url: CS_URL, version: '1.0.0', status: 'active', content: 'complete', + concept: [...codes, 'unlisted'].map(code => ({ code, display: 'Display ' + code })) +}; +const VS = { + resourceType: 'ValueSet', url: VS_URL, version: '1.0.0', status: 'active', + compose: { include: [{ system: CS_URL, concept: codes.map(code => ({ code })) }] } +}; + +async function validate(app, code) { + return request(app).post('/tx/r5/ValueSet/$validate-code') + .set('Content-Type', 'application/json') + .send({ resourceType: 'Parameters', parameter: [ + { name: 'tx-resource', resource: CS }, + { name: 'valueSet', resource: VS }, + { name: 'coding', valueCoding: { system: CS_URL, code } } + ] }); +} + +const resultOf = (res) => res.body.parameter.find(p => p.name === 'result').valueBoolean; + +describe('$validate-code: a miss against an enumerated include', () => { + let app; + beforeAll(async () => { app = await getTestApp(); }, 60000); + afterAll(async () => { await shutdownTestApp(); }); + + test.each([ + ['in the code system but not listed', 'unlisted'], + ['not in the code system', 'no-such-code'] + ])('a code %s is located a handful of times, not once per listed code', async (_label, code) => { + const spy = jest.spyOn(FhirCodeSystemProvider.prototype, 'locate'); + try { + const res = await validate(app, code); + expect(res.status).toBe(200); + expect(resultOf(res)).toBe(false); + const lookupsOfCode = spy.mock.calls.filter(c => c[0] === code).length; + expect(lookupsOfCode).toBeLessThan(10); + } finally { + spy.mockRestore(); + } + }); + + test('a listed code still validates', async () => { + const res = await validate(app, 'c150'); + expect(res.status).toBe(200); + expect(resultOf(res)).toBe(true); + }); +}); + +/** + * otherFormMatch against the loop it replaces, on a code system with two spellings per concept + * (as ICD-11 has): an entity id, and for most concepts a short code. The old loop called + * sameConceptDifferentForm for each listed concept in turn and took the first that said yes. + */ +describe('otherFormMatch gives what the old loop gave', () => { + // canonical code -> short code (null: no short code, so only one spelling) + const concepts = { 100: '1A00', 200: '1B00', 300: null, 400: '1D00' }; + const byForm = new Map(); + for (const [id, short] of Object.entries(concepts)) { + byForm.set(id, { id, short }); + if (short) byForm.set(short, { id, short }); + } + const fakeCs = () => ({ + locate: jest.fn(async (code) => ({ context: byForm.get(code) || null })), + classificationCode: async (ctxt) => ctxt.short, + code: async (ctxt) => ctxt.id + }); + const checker = Object.create(ValueSetChecker.prototype); + checker.worker = { deadCheck: () => {} }; + + async function oldLoop(cs, code, list) { + if (list && !list.some(cc => cc.code === code)) { + for (const cc of list) { + if (await checker.sameConceptDifferentForm(cs, code, cc.code)) { + return cc; + } + } + } + return null; + } + + // every code as asked (each spelling, and unknown ones) against many lists of listed codes + const asked = [...byForm.keys(), 'nope', '9Z99']; + let seed = 7; + const rnd = (n) => { seed = (seed * 1103515245 + 12345) % 2147483648; return seed % n; }; + const lists = [[]]; + for (let i = 0; i < 200; i++) { + lists.push(Array.from({ length: 1 + rnd(6) }, () => ({ code: asked[rnd(asked.length)] }))); + } + + test('the same listed concept (or none) for every code and list', async () => { + let matches = 0; + for (const code of asked) { + for (const list of lists) { + const expected = await oldLoop(fakeCs(), code, list); + const actual = await checker.otherFormMatch(fakeCs(), code, list); + expect(actual).toBe(expected); + if (actual) matches++; + } + } + expect(matches).toBeGreaterThan(50); // the comparison exercised real matches, not just misses + }); + + test('a code with one spelling is located once, however long the list', async () => { + const list = Array.from({ length: 1000 }, (_, i) => ({ code: 'x' + i })); + for (const code of ['300', 'nope']) { + const cs = fakeCs(); + expect(await checker.otherFormMatch(cs, code, list)).toBeNull(); + expect(cs.locate).toHaveBeenCalledTimes(1); + } + }); + + test('an empty list locates nothing, as before', async () => { + const cs = fakeCs(); + expect(await checker.otherFormMatch(cs, '100', [])).toBeNull(); + expect(cs.locate).not.toHaveBeenCalled(); + }); +}); diff --git a/tx/workers/validate.js b/tx/workers/validate.js index d4b5a8d6..e637afbc 100644 --- a/tx/workers/validate.js +++ b/tx/workers/validate.js @@ -1618,6 +1618,30 @@ class ValueSetChecker { return !!b.context && (await cs.code(a.context)) === (await cs.code(b.context)); } + /** + * The listed concept that is the code written in its other form (ICD-11's short code and + * entity id), or null. sameConceptDifferentForm asks two things of each listed concept: does + * the code have another form at all (locate the code, ask for its classification code), and + * is this listed concept that form. The first depends only on the code, so it is asked once + * here, and the listed concepts are compared only when the answer is yes. + */ + async otherFormMatch(cs, code, concepts) { + if (!concepts || concepts.length === 0 || concepts.some(cc => cc.code === code)) { + return null; + } + const a = await cs.locate(code); + if (!a.context || !(await cs.classificationCode(a.context))) { + return null; + } + for (const cc of concepts) { + this.worker.deadCheck('checkConceptSet#3'); + if (await this.sameConceptDifferentForm(cs, code, cc.code)) { + return cc; + } + } + return null; + } + async noteClassificationCode(cs, context, code, normalForm) { // the code system decides whether this spelling of the code earns a classificationCode // (ICD-11: when it was written with entity ids, in any order - not when it was written @@ -1726,15 +1750,7 @@ class ValueSetChecker { } // an enumerated concept may be written in the other form (ICD-11 short code vs uri) - let altMatch = null; - if (cset.concept && !cset.concept.some(cc => cc.code === code)) { - for (const cc of cset.concept) { - if (await this.sameConceptDifferentForm(cs, code, cc.code)) { - altMatch = cc; - break; - } - } - } + const altMatch = await this.otherFormMatch(cs, code, cset.concept); for (let cc of cset.concept || []) { this.worker.deadCheck('checkConceptSet#1'); let c = cc.code;