Merge pull request #265 from HealthIntersections/2026-07-gg-validation-cleanup

grahamegrieve · web-flow · commit e39fe802b5d3 · 2026-07-02T08:34:48.000+10:00
2026 07 gg validation cleanup
diff --git 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/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/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,31 +55,70 @@ 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<string>} 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) {
         if (p.name == 'validation') {
           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
    */
diff --git a/tx/workers/validate.js b/tx/workers/validate.js
