From e76352ccb703ff1237a0f9bbda64484c37cd812d Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Wed, 1 Jul 2026 17:39:12 +1000 Subject: [PATCH 1/2] rework batch processing for caches --- tests/tx/cache-control.test.js | 170 +++++++++++++++++++++++++++++++++ tx/workers/batch-validate.js | 143 +++++++++++++++++++++++---- 2 files changed, 295 insertions(+), 18 deletions(-) diff --git a/tests/tx/cache-control.test.js b/tests/tx/cache-control.test.js index af07933d..9a71c94d 100644 --- a/tests/tx/cache-control.test.js +++ b/tests/tx/cache-control.test.js @@ -422,4 +422,174 @@ describe('$cache-control routing (scaffolding)', () => { expect(codes).toContain('b1'); }); }); + + // ---- batch front-loading (two-pass) ---- + // + // A batch against an unsealed cache front-loads every resource it supplies + // (tx-resource + primary valueSet/codeSystem) into the cache before any entry is + // evaluated. So the batch is order-independent (an entry may reference by url a + // resource a later entry supplies) and a failing entry does not withhold what it + // carried. A sealed cache does not grow, so there is no cross-entry sharing. + describe('batch front-loading (two-pass)', () => { + const BATCH = '/tx/r5/ValueSet/$batch-validate-code'; + + const entry = (parameter) => ({ + name: 'validation', + resource: { resourceType: 'Parameters', parameter } + }); + const results = (body) => (body.parameter || []).filter(x => x.name === 'validation'); + + async function startCache(sealed) { + const parameter = []; + if (sealed !== undefined) parameter.push({ name: 'sealed', valueBoolean: sealed }); + const started = await request(app).post(BASE).query({ mode: 'start' }) + .set('Content-Type', 'application/json') + .send({ resourceType: 'Parameters', parameter }); + return cacheIdFrom(started.body); + } + + const mkCS = (tag) => ({ + resourceType: 'CodeSystem', url: `http://example.org/batch/${tag}-cs`, + version: '1.0.0', status: 'active', content: 'complete', + concept: [{ code: `${tag}1`, display: `${tag} one` }] + }); + const mkVS = (tag, cs) => ({ + resourceType: 'ValueSet', url: `http://example.org/batch/${tag}-vs`, + version: '1.0.0', status: 'active', + compose: { include: [{ system: cs.url }] } + }); + + test('an entry resolves a url supplied only by a LATER entry (unsealed)', async () => { + const cacheId = await startCache(false); + const cs = mkCS('fwd'); const vs = mkVS('fwd', cs); + const res = await request(app).post(BATCH) + .set('Content-Type', 'application/json') + .set('x-cache-id', cacheId) + .send({ resourceType: 'Parameters', parameter: [ + // entry 0: references vs by url only (forward reference) + entry([ + { name: 'url', valueString: vs.url }, + { name: 'coding', valueCoding: { system: cs.url, code: 'fwd1' } } + ]), + // entry 1: supplies vs + cs inline, AFTER the entry that references them + entry([ + { name: 'tx-resource', resource: cs }, + { name: 'valueSet', resource: vs }, + { name: 'coding', valueCoding: { system: cs.url, code: 'fwd1' } } + ]) + ] }); + expect(res.status).toBe(200); + const rs = results(res.body); + expect(rs.length).toBe(2); + // the forward-referencing entry validated true because pass 1 pooled the + // resources from the later entry before any entry ran. + const r0 = (rs[0].resource.parameter || []).find(x => x.name === 'result'); + expect(r0 && r0.valueBoolean).toBe(true); + }); + + test('resources are front-loaded even when the carrying entry fails, and persist (unsealed)', async () => { + const cacheId = await startCache(false); + const cs = mkCS('fail'); const vs = mkVS('fail', cs); + const batch = await request(app).post(BATCH) + .set('Content-Type', 'application/json') + .set('x-cache-id', cacheId) + .send({ resourceType: 'Parameters', parameter: [ + // this entry supplies vs+cs but validates a code that isn't in the system + entry([ + { name: 'tx-resource', resource: cs }, + { name: 'valueSet', resource: vs }, + { name: 'coding', valueCoding: { system: cs.url, code: 'NOPE' } } + ]) + ] }); + expect(batch.status).toBe(200); + + // Despite that entry not validating cleanly, vs was populated: a separate + // by-reference $expand on the same cache now resolves it. + const exp = await request(app).post('/tx/r5/ValueSet/$expand') + .set('Content-Type', 'application/json') + .set('x-cache-id', cacheId) + .send({ resourceType: 'Parameters', parameter: [{ name: 'url', valueUri: vs.url }] }); + expect(exp.status).toBe(200); + const codes = ((exp.body.expansion || {}).contains || []).map(c => c.code); + expect(codes).toContain('fail1'); + }); + + test('a sealed batch does NOT share resources across entries', async () => { + const cacheId = await startCache(true); + const cs = mkCS('seal'); const vs = mkVS('seal', cs); + const res = await request(app).post(BATCH) + .set('Content-Type', 'application/json') + .set('x-cache-id', cacheId) + .send({ resourceType: 'Parameters', parameter: [ + // entry 0 references vs by url only + entry([ + { name: 'url', valueString: vs.url }, + { name: 'coding', valueCoding: { system: cs.url, code: 'seal1' } } + ]), + // entry 1 supplies vs - but a sealed cache does not share it to entry 0 + entry([ + { name: 'tx-resource', resource: cs }, + { name: 'valueSet', resource: vs }, + { name: 'coding', valueCoding: { system: cs.url, code: 'seal1' } } + ]) + ] }); + expect(res.status).toBe(200); + const rs = results(res.body); + // entry 0 could not resolve vs (no cross-entry sharing when sealed): + // either an OperationOutcome or result=false, but not a clean true. + const r0res = rs[0].resource; + const r0 = (r0res.parameter || []).find(x => x.name === 'result'); + const unresolved = r0res.resourceType === 'OperationOutcome' || (r0 && r0.valueBoolean === false); + expect(unresolved).toBe(true); + // entry 1, which carried the resource itself, still validates true. + const r1 = (rs[1].resource.parameter || []).find(x => x.name === 'result'); + expect(r1 && r1.valueBoolean).toBe(true); + }); + + test('an unknown cache-id fails the whole batch with a coded 404', async () => { + const cs = mkCS('unk'); const vs = mkVS('unk', cs); + const res = await request(app).post(BATCH) + .set('Content-Type', 'application/json') + .set('x-cache-id', 'never-issued-this-id') + .send({ resourceType: 'Parameters', parameter: [ + entry([ + { name: 'valueSet', resource: vs }, + { name: 'coding', valueCoding: { system: cs.url, code: 'unk1' } } + ]) + ] }); + expect(res.status).toBe(404); + expect(res.body.resourceType).toBe('OperationOutcome'); + const coding = (((res.body.issue || [])[0] || {}).details || {}).coding || []; + expect(coding.some(c => c.code === 'cache-id-unknown')).toBe(true); + }); + + // CodeSystem batch: same front-loading, but the primary being validated is a + // code system (system+code), not a value set. + test('a CodeSystem batch front-loads and resolves a system supplied by a later entry (unsealed)', async () => { + const CS_BATCH = '/tx/r5/CodeSystem/$batch-validate-code'; + const cacheId = await startCache(false); + const cs = mkCS('csbatch'); + const res = await request(app).post(CS_BATCH) + .set('Content-Type', 'application/json') + .set('x-cache-id', cacheId) + .send({ resourceType: 'Parameters', parameter: [ + // entry 0: validates a code against cs by system url only (forward ref) + entry([ + { name: 'system', valueUri: cs.url }, + { name: 'code', valueCode: 'csbatch1' } + ]), + // entry 1: supplies cs inline, AFTER the entry that references it + entry([ + { name: 'tx-resource', resource: cs }, + { name: 'system', valueUri: cs.url }, + { name: 'code', valueCode: 'csbatch1' } + ]) + ] }); + expect(res.status).toBe(200); + const rs = results(res.body); + expect(rs.length).toBe(2); + const r0 = (rs[0].resource.parameter || []).find(x => x.name === 'result'); + expect(r0 && r0.valueBoolean).toBe(true); + }); + }); }); diff --git a/tx/workers/batch-validate.js b/tx/workers/batch-validate.js index d66e36dd..12b1ae8a 100644 --- a/tx/workers/batch-validate.js +++ b/tx/workers/batch-validate.js @@ -1,14 +1,15 @@ // -// Validate Worker - Handles $validate-code operations +// Batch Validate Worker - Handles $batch-validate-code operations // -// GET /CodeSystem/$validate-code?{params} -// POST /CodeSystem/$validate-code -// GET /CodeSystem/{id}/$validate-code?{params} -// POST /CodeSystem/{id}/$validate-code -// GET /ValueSet/$validate-code?{params} -// POST /ValueSet/$validate-code -// GET /ValueSet/{id}/$validate-code?{params} -// POST /ValueSet/{id}/$validate-code +// GET/POST /ValueSet/$batch-validate-code - batch of ValueSet $validate-code +// GET/POST /CodeSystem/$batch-validate-code - batch of CodeSystem $validate-code +// +// A batch is a Parameters whose `validation` entries each carry a single +// $validate-code request (as a nested Parameters). Shared inputs may be supplied +// once at the top level ("globals") and are applied to every entry. Entries are +// dispatched per-shape (a url/valueSet entry validates against a value set; a +// system/codeSystem entry validates against a code system), so both batch routes +// share one implementation and even a mixed batch is handled correctly. // const { TerminologyWorker } = require('./worker'); @@ -30,12 +31,20 @@ class BatchValidateWorker extends TerminologyWorker { */ constructor(opContext, log, provider, languages, i18n) { super(opContext, log, provider, languages, i18n); - this.globalNames.add("tx-resource"); - this.globalNames.add("url"); - this.globalNames.add("valueSet"); - this.globalNames.add("lenient-display-validation"); - this.globalNames.add("__Accept-Language"); - this.globalNames.add("__Content-Language"); + // "Global" parameters may be supplied once at the top level and are applied to + // every entry that doesn't override them. tx-resource is always shared. The + // primary differs by operation: a ValueSet batch shares url/valueSet, a + // CodeSystem batch shares system/codeSystem. + this.valueSetGlobalNames = new Set([ + "tx-resource", "url", "valueSet", "lenient-display-validation", + "__Accept-Language", "__Content-Language" + ]); + this.codeSystemGlobalNames = new Set([ + "tx-resource", "system", "codeSystem", "lenient-display-validation", + "__Accept-Language", "__Content-Language" + ]); + // Back-compat: `globalNames` was the (ValueSet) set before CodeSystem batches. + this.globalNames = this.valueSetGlobalNames; } /** @@ -46,18 +55,43 @@ class BatchValidateWorker extends TerminologyWorker { return 'batch-validate-code'; } + /** ValueSet/$batch-validate-code: shared globals are url/valueSet. */ async handleValueSet(req, res) { + return this.processBatch(req, res, this.valueSetGlobalNames); + } + + /** CodeSystem/$batch-validate-code: shared globals are system/codeSystem. */ + async handleCodeSystem(req, res) { + return this.processBatch(req, res, this.codeSystemGlobalNames); + } + + /** + * Run a batch of $validate-code requests. Two passes: frontLoadBatch() pools all + * supplied resources into an unsealed cache first (see that method); then each + * `validation` entry is evaluated, inheriting the top-level globals it does not + * override and dispatched by shape to the ValueSet or CodeSystem validator. + * + * @param {express.Request} req + * @param {express.Response} res + * @param {Set} globalNames - which top-level params are shared globals + */ + async processBatch(req, res, globalNames) { try { let params = req.body; this.addHttpParams(req, params); let globalParams = []; for (const p of params.parameter) { - if (this.globalNames.has(p.name)) { + if (globalNames.has(p.name)) { globalParams.push(p); } } + // Pass 1: front-load every resource the batch supplies into the (unsealed) + // session cache before any entry is evaluated. When this happens, pass 2 + // below drops per-entry tx-resource processing (they're already cached). + const frontLoaded = this.frontLoadBatch(params); + let output = []; for (const p of params.parameter) { @@ -65,12 +99,26 @@ class BatchValidateWorker extends TerminologyWorker { let op = new Parameters(); op.jsonObj.parameter = []; for (const gp of globalParams) { + if (gp.name == 'tx-resource') { + // Pass 2: when the batch was front-loaded into an unsealed cache, the + // tx-resources are already in the cache and every entry resolves them + // by reference - don't re-inject or re-process them per entry. Without + // front-loading (no cache, or a sealed cache) keep the original + // behaviour: the global tx-resources apply to every entry. + if (!frontLoaded) { + op.jsonObj.parameter.push(gp); + } + continue; + } let exists = p.resource.parameter.find(pp => gp.name == pp.name); - if (gp.name == 'tx-resource' || !exists) { + if (!exists) { op.jsonObj.parameter.push(gp); } } - op.jsonObj.parameter.push(...p.resource.parameter); + const entryParams = frontLoaded + ? p.resource.parameter.filter(pp => pp.name !== 'tx-resource') + : p.resource.parameter; + op.jsonObj.parameter.push(...entryParams); let worker = new ValidateWorker(this.opContext.copy(), this.log, this.provider, this.languages, this.i18n); try { @@ -100,11 +148,70 @@ class BatchValidateWorker extends TerminologyWorker { } catch (error) { this.log.error(error); debugLog(error); + // A batch-level failure (e.g. an unknown cache-id from pass 1) applies to the + // whole batch. Preserve a coded Issue (like cache-id-unknown) so the client + // gets the same coded OperationOutcome + status it would on a single op. + if (error instanceof Issue) { + const oo = new OperationOutcome(); + oo.addIssue(error); + return res.status(error.statusCode || 500).json(oo.jsonObj); + } return res.status(error.statusCode || 500).json(this.operationOutcome( 'error', error.issueCode || 'exception', error.message)); } } + /** + * Pass 1 of batch processing: front-load every resource the batch supplies + * (each `tx-resource`, plus each entry's primary `valueSet`/`codeSystem`) into + * the session cache before any entry is evaluated. This makes the batch + * order-independent - an entry may refer by url to a resource another entry + * supplied - and means a failing entry does not withhold the resources it + * carried, because population happens up front and as one step. + * + * Front-loading is only relevant for an *unsealed* cache: that is what grows, so + * that is how one entry's resources become visible to the others. A sealed cache + * does not grow (each entry stays self-contained), and with no cache there is + * nowhere to pool; in both cases this returns false and pass 2 keeps the original + * per-entry tx-resource handling. + * + * The unknown-cache-id check is done here, once, for the whole batch. + * + * @param {Object} params - the batch Parameters (req.body) + * @returns {boolean} true if resources were front-loaded into an unsealed cache + */ + frontLoadBatch(params) { + const cacheId = this.opContext ? this.opContext.cacheId : null; + const cache = this.opContext ? this.opContext.resourceCache : null; + if (!cacheId || !cache) { + return false; + } + if (!cache.has(cacheId)) { + throw new Issue('error', 'not-found', null, 'CACHE_ID_UNKNOWN', + this.i18n.translate('CACHE_ID_UNKNOWN', this.opContext.langs, [cacheId]), + 'cache-id-unknown', 404); + } + if (cache.isSealed(cacheId)) { + return false; + } + // Flatten the top-level params and every entry's params into one list, then let + // collectSuppliedResources pick out the tx-resource + primary valueSet/codeSystem. + const allParams = []; + for (const p of params.parameter) { + if (p.name === 'validation' && p.resource && Array.isArray(p.resource.parameter)) { + allParams.push(...p.resource.parameter); + } else { + allParams.push(p); + } + } + const { txResources, primaryResources } = this.collectSuppliedResources({ parameter: allParams }); + const pool = txResources.concat(primaryResources); + if (pool.length > 0) { + cache.add(cacheId, pool); + } + return true; + } + /** * Build an OperationOutcome */ From f3ddd3a18bcc1a1a4e8633139d01f53e7417b556 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 2 Jul 2026 03:20:24 +1000 Subject: [PATCH 2/2] Report the FIRST matching coding, not the last, and only one message about status --- translations/Messages.properties | 3 ++- tx/workers/validate.js | 37 +++++++++++++++++++------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/translations/Messages.properties b/translations/Messages.properties index b5ad9a61..ad389985 100644 --- a/translations/Messages.properties +++ b/translations/Messages.properties @@ -1492,7 +1492,8 @@ RESOURCE_INTERNAL_USE_ONLY = This {0} comes from the package {1} which has been TYPE_SPECIFIC_CHECKS_DT_SID_INCORRECT = The URL {0} is not a correct URL to use (in spite of being in the standard, and/or accepted in the past) INACTIVE_DISPLAY_FOUND_one = ''{1}'' is no longer considered a correct display for code ''{2}'' (status = {4}). The correct display is ''{3}'' INACTIVE_DISPLAY_FOUND_other = ''{1}'' is no longer considered a correct display for code ''{2}'' (status = {4}). The correct display is one of {3} -INACTIVE_CONCEPT_FOUND = The concept ''{1}'' has a status of {0} and its use should be reviewed +INACTIVE_CONCEPT_FOUND = The concept ''{1}'' has a status of {0} and its use should be reviewed +INACTIVE_CONCEPT_FOUND_ADD = The concept ''{1}'' has a status of {0} and {2} and its use should be reviewed DEPRECATED_CONCEPT_FOUND = The concept ''{1}'' is deprecated and its use should be reviewed CONCEPT_DEPRECATED_IN_VALUESET = The presence of the concept ''{1}'' in the system ''{0}'' in the value set {3} is marked with a status of {2} and its use should be reviewed SYSTEM_VERSION_MULTIPLE_OVERRIDE = Multiple version overrides found for system {0}: ''{1}'', ''{2}''); diff --git a/tx/workers/validate.js b/tx/workers/validate.js index 9fdd77f0..df16b420 100644 --- a/tx/workers/validate.js +++ b/tx/workers/validate.js @@ -1126,17 +1126,23 @@ class ValueSetChecker { } else if (c.display && list.designations.length > 0) { await this.checkDisplays(list, defLang, c, msg, op, path); } - psys = c.system; - pcode = c.code; - if (ver.value) { - pver = ver.value; - } - let pd = list.preferredDisplay(this.params.workingLanguages()); - if (pd) { - pdisp = pd; - } - if (!pdisp) { - pdisp = list.preferredDisplay(null); + // Report the FIRST matching coding, not the last: when a CodeableConcept + // has several valid codings, the code/system/version/display echoed back + // are those of the earliest coding that validated. Only capture once + // (pcode is undefined until the first match). + if (pcode === undefined) { + psys = c.system; + pcode = c.code; + if (ver.value) { + pver = ver.value; + } + let pd = list.preferredDisplay(this.params.workingLanguages()); + if (pd) { + pdisp = pd; + } + if (!pdisp) { + pdisp = list.preferredDisplay(null); + } } } else if (!this.params.membershipOnly && ws) { if (!isAbsoluteUrl(ws)) { @@ -1354,12 +1360,13 @@ class ValueSetChecker { if (!mpath) { mpath = 'code'; } + let mid1 = "INACTIVE_CONCEPT_FOUND"; + let mm1 = ""; if (!['inactive', 'DISCOURAGED'].includes(vstatus.value)) { - let m = this.worker.i18n.translate('INACTIVE_CONCEPT_FOUND', this.params.HTTPLanguages, ['inactive', tcode]); - msg(m); - op.addIssue(new Issue('warning', 'business-rule', mpath, 'INACTIVE_CONCEPT_FOUND', m, 'code-comment')); + mid1 = 'INACTIVE_CONCEPT_FOUND_ADD'; + mm1 = 'inactive'; } - let m = this.worker.i18n.translate('INACTIVE_CONCEPT_FOUND', this.params.HTTPLanguages, [vstatus.value, tcode]); + let m = this.worker.i18n.translate(mid1, this.params.HTTPLanguages, [vstatus.value, tcode, mm1]); msg(m); op.addIssue(new Issue('warning', 'business-rule', mpath, 'INACTIVE_CONCEPT_FOUND', m, 'code-comment')); } else if (vstatus.value && vstatus.value.toLowerCase() === 'deprecated') {