From 423a99928ff21b5a5297ed74dca881973f438e0f Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Sat, 13 Jun 2026 13:32:35 -0400 Subject: [PATCH] fix bug handling cache-control header --- tests/tx/cache-control.test.js | 105 +++++++++++++++++++++++++++++++++ tx/operation-context.js | 7 +++ tx/tx.js | 6 ++ tx/workers/worker.js | 12 +++- 4 files changed, 128 insertions(+), 2 deletions(-) diff --git a/tests/tx/cache-control.test.js b/tests/tx/cache-control.test.js index ab82324d..99f50f40 100644 --- a/tests/tx/cache-control.test.js +++ b/tests/tx/cache-control.test.js @@ -204,4 +204,109 @@ describe('$cache-control routing (scaffolding)', () => { .send(emptyBody); expect(res.status).toBe(200); }); + + // ---- header-driven cache use across operations (regression) ---- + // + // The cache-id travels as the X-Cache-Id header. Only the operations whose + // Parameters are assembled through buildParameters() used to honour it; the + // ones that read req.body / the query string directly (expand, related, + // batch-validate) or that handed setupAdditionalResources a raw req.body + // (lookup) silently ignored a front-loaded cache and failed to resolve + // by-reference resources. The middleware now lifts the header onto the + // operation context and setupAdditionalResources falls back to it, so every + // operation honours a front-loaded cache from the header alone (no inline + // tx-resource, no cache-id parameter). + describe('front-loaded cache is honoured from the header alone', () => { + const colorsCS = { + resourceType: 'CodeSystem', + url: 'http://example.org/hdr-test/colors', + version: '1.0.0', + status: 'active', + content: 'complete', + concept: [{ code: 'red', display: 'Red' }, { code: 'green', display: 'Green' }] + }; + const colorsVS = { + resourceType: 'ValueSet', + url: 'http://example.org/hdr-test/colors-vs', + version: '1.0.0', + status: 'active', + compose: { include: [{ system: colorsCS.url }] } + }; + + async function startCache() { + const started = await request(app) + .post(BASE) + .query({ mode: 'start' }) + .set('Content-Type', 'application/json') + .send({ + resourceType: 'Parameters', + parameter: [ + { name: 'tx-resource', resource: colorsCS }, + { name: 'valueSet', resource: colorsVS } + ] + }); + return cacheIdFrom(started.body); + } + + test('$expand resolves the front-loaded ValueSet by url via the header', async () => { + const cacheId = await startCache(); + const res = 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: colorsVS.url }] }); + expect(res.status).toBe(200); + expect(res.body.resourceType).toBe('ValueSet'); + const codes = ((res.body.expansion || {}).contains || []).map(c => c.code).sort(); + expect(codes).toEqual(['green', 'red']); + }); + + test('$lookup resolves the front-loaded CodeSystem via the header', async () => { + const cacheId = await startCache(); + const res = await request(app) + .post('/tx/r5/CodeSystem/$lookup') + .set('Content-Type', 'application/json') + .set('x-cache-id', cacheId) + .send({ + resourceType: 'Parameters', + parameter: [ + { name: 'system', valueUri: colorsCS.url }, + { name: 'code', valueCode: 'green' } + ] + }); + expect(res.status).toBe(200); + const display = (res.body.parameter || []).find(x => x.name === 'display'); + expect(display && display.valueString).toBe('Green'); + }); + + test('$validate-code resolves the front-loaded ValueSet by url via the header', async () => { + const cacheId = await startCache(); + const res = await request(app) + .post('/tx/r5/ValueSet/$validate-code') + .set('Content-Type', 'application/json') + .set('x-cache-id', cacheId) + .send({ + resourceType: 'Parameters', + parameter: [ + { name: 'url', valueString: colorsVS.url }, + { name: 'coding', valueCoding: { system: colorsCS.url, code: 'red' } } + ] + }); + expect(res.status).toBe(200); + const result = (res.body.parameter || []).find(x => x.name === 'result'); + expect(result && result.valueBoolean).toBe(true); + }); + + test('an unknown cache-id in the header is a coded 404 on $expand', async () => { + const res = await request(app) + .post('/tx/r5/ValueSet/$expand') + .set('Content-Type', 'application/json') + .set('x-cache-id', 'never-issued-this-id') + .send({ resourceType: 'Parameters', parameter: [{ name: 'url', valueUri: colorsVS.url }] }); + 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); + }); + }); }); diff --git a/tx/operation-context.js b/tx/operation-context.js index f42ec8b5..9be55b07 100644 --- a/tx/operation-context.js +++ b/tx/operation-context.js @@ -535,6 +535,12 @@ class OperationContext { this.logEntries = []; this.resourceCache = resourceCache; this.expansionCache = expansionCache; + // Server-issued cache-id carried on the request (X-Cache-Id header). Set once + // per request from the header so every worker can consult it uniformly via + // setupAdditionalResources, regardless of how that worker assembles its + // Parameters (buildParameters, raw req.body, query/form). An explicit + // cache-id *parameter* still takes precedence over this. + this.cacheId = null; this.debugging = isDebugging(); // Providers opened during this operation that need their underlying // resources (sqlite connections, etc.) released when the operation ends. @@ -568,6 +574,7 @@ class OperationContext { newContext.timeTracker = this.timeTracker.link(); newContext.logEntries = [...this.logEntries]; newContext.debugging = this.debugging; + newContext.cacheId = this.cacheId; newContext.usageTracker = this.usageTracker; // Share the same provider-cleanup list so providers opened by the copy // are released when the parent operation ends. diff --git a/tx/tx.js b/tx/tx.js index a883861c..29f59dfe 100644 --- a/tx/tx.js +++ b/tx/tx.js @@ -297,6 +297,12 @@ class TXModule { endpointInfo.resourceCache, endpointInfo.expansionCache ); opContext.usageTracker = this.usageTracker; + // Normalise the cache-id from the X-Cache-Id header onto the operation + // context once, here, so every worker honours a front-loaded cache no + // matter how it later assembles its Parameters (some use buildParameters, + // others read req.body / query directly). An explicit cache-id parameter + // on the request still wins (see setupAdditionalResources). + opContext.cacheId = req.get('X-Cache-Id') || null; // Attach everything to request req.txProvider = endpointInfo.provider; diff --git a/tx/workers/worker.js b/tx/workers/worker.js index ab28d134..0213d390 100644 --- a/tx/workers/worker.js +++ b/tx/workers/worker.js @@ -641,9 +641,17 @@ class TerminologyWorker { // primary resource is included. const { txResources, primaryResources } = this.collectSuppliedResources(params); - // Check for cache-id + // Check for cache-id. An explicit cache-id *parameter* wins; otherwise fall + // back to the cache-id the middleware lifted off the X-Cache-Id header onto + // the operation context. This fallback is what makes the header work on the + // op paths that don't route their Parameters through buildParameters + // (expand, related, batch-validate) or that hand setupAdditionalResources a + // raw req.body (lookup) - previously those silently ignored a front-loaded + // cache and failed to resolve by-reference resources. const cacheIdParam = this.findParameter(params, 'cache-id'); - const cacheId = cacheIdParam ? this.getParameterValue(cacheIdParam) : null; + const cacheId = (cacheIdParam ? this.getParameterValue(cacheIdParam) : null) + || (this.opContext ? this.opContext.cacheId : null) + || null; if (cacheId && this.opContext.resourceCache) { // The cache must already exist: caches are created explicitly via