Small fixes: requests no longer wait behind batches, one hung reply, regex alternation, and a few speed-ups

library/keep-alive.js

@@ -0,0 +1,18 @@
+// Keep-alive timeouts for the HTTP server behind a reverse proxy.
+//
+// nginx keeps idle upstream connections open for 60 s (keepalive_timeout in an upstream block),
+// while Node closes an idle one after 5 s. When nginx reuses a connection at the moment Node
+// closes it, the request fails with a reset, and nginx does not retry a POST: the client gets a
+// 502. Node must therefore keep idle connections open longer than the proxy does, and
+// headersTimeout must be longer still, or Node applies it to the idle connection first.
+
+const KEEP_ALIVE_TIMEOUT_MS = 65000;
+const HEADERS_TIMEOUT_MS = 66000;
+
+function applyKeepAliveTimeouts(server) {
+  server.keepAliveTimeout = KEEP_ALIVE_TIMEOUT_MS;
+  server.headersTimeout = HEADERS_TIMEOUT_MS;
+  return server;
+}
+
+module.exports = { applyKeepAliveTimeouts, KEEP_ALIVE_TIMEOUT_MS, HEADERS_TIMEOUT_MS };

server.js

@@ -18,6 +18,7 @@
 const { resolveWithin } = require('./library/path-safety');
 const { readCgroupMemoryLimit } = require('./library/cgroup-memory');
 const memoryGuard = require('./library/memory-guard');
+const { applyKeepAliveTimeouts } = require('./library/keep-alive');
 const { NpmAudit } = require('./library/npm-audit');
 const Mailer = require('./library/mailer');
 const DailyReport = require('./library/daily-report');
@@ -836,10 +837,12 @@
     });
 
     // Start server
-    app.listen(PORT, () => {
+    const server = app.listen(PORT, () => {
       stats.markStarted(packageJson.version);
       serverLog.info(`=== Server running on http://localhost:${PORT} ===`);
     });
+    // idle connections stay open longer than nginx keeps them (see library/keep-alive.js)
+    applyKeepAliveTimeouts(server);
     npmAudit.start();
     dailyReport = new DailyReport(config.dailyReport, {
       log: Logger.getInstance().child({ module: 'daily-report' }),
@@ -856,7 +859,7 @@
     }
     // Run usage tracker in background after startup
     if (modules.tx && modules.tx.library) {
       setImmediate(async () => {
         try {
           serverLog.info('Starting ConceptUsageTracker scan...');
           let count = await modules.tx.usageTracker.scanValueSets(modules.tx.library);

tests/cs/cs-country.test.js

@@ -261,6 +261,23 @@ describe('CountryCodeServices', () => {
       expect(results[0].code).toBe('US');
     });
 
+    test('a pattern with alternation matches whole codes only', async () => {
+      // '^' + 'US|CA' + '$' would be "starts with US or ends with CA", taking in USA
+      const ctxt = await provider.getPrepContext(false);
+      await provider.filter(ctxt, true, 'code', 'regex', 'US|CA');
+      const filters = await provider.executeFilters(ctxt);
+      const filter = filters[0];
+
+      const results = [];
+      filter.cursor = -1;
+      while (await provider.filterMore(ctxt, filter)) {
+        const concept = await provider.filterConcept(ctxt, filter);
+        results.push(concept.code);
+      }
+
+      expect(results.sort()).toEqual(['CA', 'US']);
+    });
+
     test('should filter all 2-letter codes', async () => {
       const ctxt = await provider.getPrepContext(false);
       await provider.filter(ctxt, true, 'code', 'regex', '[A-Z]{2}');

tests/cs/cs-loinc.test.js

@@ -7,6 +7,7 @@ const { OperationContext } = require('../../tx/operation-context');
 const {validateParameter} = require("../../library/utilities");
 const {Designations, SearchFilterText} = require("../../tx/library/designations");
 const {TestUtilities} = require("../test-utilities");
+const {CodeSystem} = require('../../tx/library/codesystem');
 const folders = require('../../library/folder-setup');
 
 describe('LOINC Module Import', () => {
@@ -916,6 +917,33 @@ describe('LOINC Provider', () => {
       expect(size).toBeGreaterThanOrEqual(0);
     });
 
+    // LOINC's own text search needs every word (no display in the test data has "serum
+    // glucose"); a supplement's designations must match the same way, not on any one word
+    test('a supplement\'s designations match a text filter only when they have every word', async () => {
+      const supplement = new CodeSystem({
+        resourceType: 'CodeSystem', url: 'http://example.org/loinc-supplement', version: '1', status: 'active',
+        content: 'supplement', supplements: 'http://loinc.org', language: 'en',
+        concept: [
+          { code: '101448-9', designation: [{ language: 'en', value: 'Serum antibody screen' }] },
+          { code: '104597-0', display: 'Blood glucose by meter' },
+          { code: '102820-8', designation: [{ language: 'en', value: 'Glucose level in serum' }] }
+        ]
+      });
+      const supplemented = await factory.build(opContext, [supplement]);
+      try {
+        const filterContext = await supplemented.getPrepContext(true);
+        await supplemented.searchFilter(filterContext, new SearchFilterText('serum glucose'), false);
+        const filters = await supplemented.executeFilters(filterContext);
+        const codes = [];
+        while (await supplemented.filterMore(filterContext, filters[0])) {
+          codes.push(await supplemented.code(await supplemented.filterConcept(filterContext, filters[0])));
+        }
+        expect(codes).toEqual(['102820-8']);
+      } finally {
+        supplemented.close();
+      }
+    });
+
     test('lists text filter matches in description order, sorted or not', async () => {
       for (const sort of [false, true]) {
         const filterContext = await provider.getPrepContext(true);
@@ -1097,6 +1125,38 @@ describe('LOINC Provider', () => {
       ]);
     });
 
+    // The importer stores class type 3 as 'Claims attachment' (1,161 codes in LOINC 2.83, such
+    // as 18719-5). The test data has none, so one code is given it here for the test
+    test('CLASSTYPE 3 (Claims attachment): $lookup reports it, and the CLASSTYPE = 3 filter finds it', async () => {
+      const code = '104597-0';
+      const rw = new sqlite3.Database(testDbPath);
+      const run = (sql, args = []) => new Promise((resolve, reject) => rw.run(sql, args, function (err) { err ? reject(err) : resolve(this); }));
+      const get = (sql, args = []) => new Promise((resolve, reject) => rw.get(sql, args, (err, row) => err ? reject(err) : resolve(row)));
+      const prop = await get(`SELECT Properties.PropertyKey, Properties.PropertyValueKey FROM Properties, PropertyTypes, Codes
+        WHERE Properties.PropertyTypeKey = PropertyTypes.PropertyTypeKey AND PropertyTypes.Description = 'CLASSTYPE'
+          AND Properties.CodeKey = Codes.CodeKey AND Codes.Code = ?`, [code]);
+      const added = await run(`INSERT INTO PropertyValues (Value) VALUES ('Claims attachment')`);
+      try {
+        await run('UPDATE Properties SET PropertyValueKey = ? WHERE PropertyKey = ?', [added.lastID, prop.PropertyKey]);
+
+        const params = [];
+        await provider.extendLookup(code, ['CLASSTYPE'], params);
+        const classType = params.find(p => p.name === 'property' && p.part.find(x => x.name === 'code').valueCode === 'CLASSTYPE');
+        expect(classType.part.find(x => x.name === 'value').valueString).toBe('3');
+        expect(classType.part.find(x => x.name === 'description').valueString).toBe('Claims attachment');
+
+        const filterContext = await provider.getPrepContext(true);
+        await provider.filter(filterContext, true, 'CLASSTYPE', '=', '3');
+        const filters = await provider.executeFilters(filterContext);
+        expect(await provider.filterSize(filterContext, filters[0])).toBe(1);
+        expect((await provider.filterLocate(filterContext, filters[0], code)).code).toBe(code);
+      } finally {
+        await run('UPDATE Properties SET PropertyValueKey = ? WHERE PropertyKey = ?', [prop.PropertyValueKey, prop.PropertyKey]);
+        await run('DELETE FROM PropertyValues WHERE PropertyValueKey = ?', [added.lastID]);
+        await new Promise(resolve => rw.close(resolve));
+      }
+    });
+
     test('should extend lookup with properties and relationships', async () => {
       const testCode = expectedResults.basic.knownCodes[0];
       const paramSet = [];

tests/cs/cs-snomed.test.js

@@ -1526,6 +1526,31 @@ describe('SNOMED CT descendent-leaf and is-not-a filters', () => {
     expect(isA.length + notA.length).toBe(provider.sct.concepts.count());
     expect(notA.filter(c => isA.includes(c))).toEqual([]);
   });
+
+  // 41000147108 is from the Dutch edition, which the test edition doesn't include. As an
+  // unknown code does in other code systems, it selects nothing (is-not-a: everything) in an
+  // expansion. (doesFilter still says no to it, so $validate-code reports the filter as not
+  // understood where it asks.)
+  test('a concept the edition does not have selects no concepts, rather than failing', async () => {
+    const unknown = '41000147108';
+    expect(provider.sct.concepts.findConcept(provider.sct.stringToIdOrZero(unknown)).found).toBe(false);
+    const iterated = async (op, value) => {
+      const fc = await provider.getPrepContext(true);
+      await provider.filter(fc, true, 'concept', op, value);
+      const [set] = await provider.executeFilters(fc);
+      const out = [];
+      while (await provider.filterMore(fc, set)) {
+        out.push(await provider.code(await provider.filterConcept(fc, set)));
+      }
+      return out;
+    };
+    for (const op of ['=', 'is-a', 'descendent-of', 'child-of', 'generalizes', 'descendent-leaf', 'in']) {
+      expect(await iterated(op, unknown)).toEqual([]);
+      expect(await located(op, unknown, '16763008')).toBe(false);
+    }
+    expect(await iterated('in', unknown + ',16763008')).toEqual(['16763008']);
+    expect((await iterated('is-not-a', unknown)).length).toBe(provider.sct.concepts.count());
+  });
 });
 
 // display regex (S6): a concept matches when it has an active description (FSN or synonym,

tests/library/keep-alive.test.js

@@ -0,0 +1,40 @@
+// Idle connections must outlive the reverse proxy's (nginx: 60 s), or nginx reuses a connection
+// Node is closing and a POST on it fails with a 502. Node's default is 5 s.
+
+const http = require('http');
+const net = require('net');
+const { applyKeepAliveTimeouts, KEEP_ALIVE_TIMEOUT_MS, HEADERS_TIMEOUT_MS } = require('../../library/keep-alive');
+
+describe('keep-alive timeouts', () => {
+  test('idle connections stay open longer than nginx keeps them, and headersTimeout is longer still', () => {
+    const server = applyKeepAliveTimeouts(http.createServer());
+    expect(server.keepAliveTimeout).toBe(KEEP_ALIVE_TIMEOUT_MS);
+    expect(server.keepAliveTimeout).toBeGreaterThan(60000);
+    expect(server.headersTimeout).toBe(HEADERS_TIMEOUT_MS);
+    expect(server.headersTimeout).toBeGreaterThan(server.keepAliveTimeout);
+  });
+
+  test('a connection idle for longer than Node\'s 5 s default can still be reused', async () => {
+    const server = applyKeepAliveTimeouts(http.createServer((req, res) => res.end('ok')));
+    await new Promise(resolve => server.listen(0, '127.0.0.1', resolve));
+    const socket = net.connect(server.address().port, '127.0.0.1');
+    let closed = false;
+    socket.on('close', () => { closed = true; });
+    const send = () => new Promise((resolve, reject) => {
+      socket.once('data', d => resolve(d.toString()));
+      socket.once('error', reject);
+      socket.once('close', () => reject(new Error('connection closed')));
+      socket.write('GET / HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n');
+    });
+    try {
+      expect(await send()).toMatch(/^HTTP\/1.1 200/);
+      await new Promise(resolve => setTimeout(resolve, 6000));
+      expect(closed).toBe(false);
+      expect(await send()).toMatch(/^HTTP\/1.1 200/);
+    } finally {
+      socket.destroy();
+      server.closeAllConnections();
+      await new Promise(resolve => server.close(resolve));
+    }
+  }, 15000);
+});

tests/tx/batch-validate-yield.test.js

@@ -0,0 +1,42 @@
+/**
+ * $batch-validate-code gives the event loop a turn between entries.
+ *
+ * Validating one entry never yields by itself, so a 1,000-entry batch from an IG build used to
+ * hold the event loop for its whole duration (~1 s on production hardware), and every other
+ * request arriving meanwhile waited for it (a single lookup typically waited 0.4-0.9 s).
+ * processBatch now calls checkAndYield before each entry; checkAndYield only actually yields
+ * after YIELD_INTERVAL ms of compute, so small batches are unaffected.
+ */
+const request = require('supertest');
+const { getTestApp, shutdownTestApp } = require('./setup');
+const { OperationContext } = require('../../tx/operation-context');
+
+const CS_URL = 'http://example.org/fhir/CodeSystem/batch-yield-cs';
+const CS = {
+  resourceType: 'CodeSystem', url: CS_URL, version: '1.0.0', status: 'active', content: 'complete',
+  concept: [{ code: 'a', display: 'Alpha' }, { code: 'b', display: 'Beta' }]
+};
+const entry = (code) => ({ name: 'validation', resource: { resourceType: 'Parameters', parameter: [{ name: 'code', valueCode: code }] } });
+
+describe('$batch-validate-code yields between entries', () => {
+  let app;
+  beforeAll(async () => { app = await getTestApp(); }, 60000);
+  afterAll(async () => { await shutdownTestApp(); });
+
+  test('checkAndYield is called once per entry and the results are unchanged', async () => {
+    const spy = jest.spyOn(OperationContext.prototype, 'checkAndYield');
+    try {
+      const codes = ['a', 'zz', 'b', 'a', 'b'];
+      const res = await request(app).post('/tx/r5/CodeSystem/$batch-validate-code')
+        .set('Content-Type', 'application/json')
+        .send({ resourceType: 'Parameters', parameter: [{ name: 'tx-resource', resource: CS }, { name: 'url', valueUri: CS_URL }, ...codes.map(entry)] });
+      expect(res.status).toBe(200);
+      const calls = spy.mock.calls.filter((c) => c[0] === 'batch-validate').length;
+      expect(calls).toBe(codes.length);
+      const results = res.body.parameter.map((r) => r.resource.parameter.find((p) => p.name === 'result').valueBoolean);
+      expect(results).toEqual([true, false, true, true, true]);
+    } finally {
+      spy.mockRestore();
+    }
+  });
+});

tests/tx/cache-id.test.js

@@ -200,6 +200,17 @@ describe('cache-id protocol (integration)', () => {
       expect(isUnknownCacheError(res)).toBe(true);
     });
 
+    test('CodeSystem/$validate-code with an unknown cache-id is answered (it used to never respond)', async () => {
+      const res = await request(app).post('/tx/r5/CodeSystem/$validate-code').timeout(10000)
+        .set('Content-Type', 'application/json')
+        .send({ resourceType: 'Parameters', parameter: [
+          { name: 'cache-id', valueId: 'never-issued-this-cache' },
+          { name: 'coding', valueCoding: { system: 'http://hl7.org/fhir/administrative-gender', code: 'male' } }
+        ] });
+      expect(res.status).toBe(404);
+      expect(isUnknownCacheError(res)).toBe(true);
+    });
+
     test('by url with no cache-id and a VS not in the library fails (not a silent pass)', async () => {
       const r = await post(app, VALIDATE, [
         { name: 'url', valueString: colorsVS.url },

tests/tx/codesystem-map.test.js

@@ -0,0 +1,42 @@
+/**
+ * CodeSystemMap keeps an index of supplements by the URL they supplement, maintained on every
+ * set/delete, so Provider.loadSupplements() does not scan every loaded CodeSystem per lookup.
+ */
+const { CodeSystemMap } = require('../../tx/provider');
+
+const cs = (url, version, extra = {}) => ({ url, version, vurl: url + '|' + version, jsonObj: { url, version, content: 'complete', ...extra } });
+const supp = (url, version, of) => cs(url, version, { content: 'supplement', supplements: of });
+
+describe('CodeSystemMap supplement index', () => {
+  test('indexes supplements by the supplemented URL, without its version', () => {
+    const m = new CodeSystemMap();
+    m.set('http://x/base', cs('http://x/base', '1'));
+    m.set('http://x/s1', supp('http://x/s1', '1', 'http://x/base'));
+    m.set('http://x/s2|2', supp('http://x/s2', '2', 'http://x/base|1'));
+    m.set('http://x/other', supp('http://x/other', '1', 'http://y/base'));
+    expect([...m.supplementsOf('http://x/base')].map((c) => c.url).sort()).toEqual(['http://x/s1', 'http://x/s2']);
+    expect([...m.supplementsOf('http://y/base')].map((c) => c.url)).toEqual(['http://x/other']);
+    expect([...m.supplementsOf('http://z')]).toEqual([]);
+  });
+
+  test('replacing or deleting an entry keeps the index right', () => {
+    const m = new CodeSystemMap();
+    m.set('k', supp('http://x/s1', '1', 'http://x/base'));
+    m.set('k', supp('http://x/s1', '2', 'http://x/other'));
+    expect([...m.supplementsOf('http://x/base')]).toEqual([]);
+    expect([...m.supplementsOf('http://x/other')].map((c) => c.version)).toEqual(['2']);
+    m.delete('k');
+    expect([...m.supplementsOf('http://x/other')]).toEqual([]);
+    expect(m.supplementIndex.size).toBe(0);
+    m.set('a', supp('http://x/s1', '1', 'http://x/base'));
+    m.clear();
+    expect([...m.supplementsOf('http://x/base')]).toEqual([]);
+  });
+
+  test('behaves as a Map', () => {
+    const m = new CodeSystemMap([['a', cs('http://x/a', '1')]]);
+    expect(m.get('a').url).toBe('http://x/a');
+    expect(m.size).toBe(1);
+    expect(m instanceof Map).toBe(true);
+  });
+});

tests/tx/designations.test.js

@@ -615,6 +615,16 @@ describe('Designations', () => {
       expect(filter.passes('cardiac rhythm')).toBe(false);
     });
 
+    test('passesAll needs every word of the filter, passes only one', () => {
+      const filter = new SearchFilterText('serum glucose');
+
+      expect(filter.passes('Cholesterol in Serum')).toBe(true);
+      expect(filter.passesAll('Cholesterol in Serum')).toBe(false);
+      expect(filter.passesAll('Glucose [Mass/volume] in Serum or Plasma')).toBe(true);
+      expect(filter.passesAll('Glucoses, serum')).toBe(true); // stemmed, in any order
+      expect(new SearchFilterText('').passesAll('anything')).toBe(true);
+    });
+
     test('should calculate match scores', () => {
       const filter = new SearchFilterText('blood pressure');
       const stems = ['blood', 'pressure', 'measurement'];