Serve the species lists from node

deploy/nginx/common/routes.conf

@@ -48,6 +48,22 @@ location = /ContentService/data/content/contributors {
     proxy_pass http://content_node;
 }
 
+# The species lists, byte-identical to Java and verified by the diff harness
+# over the whole corpus rather than by eye.
+#
+# Exact matches for the same reason as the three above, and a sharper one here:
+# Java serves other shapes under /data/species/ that node does not, so a prefix
+# would claim them and 404 what works today.
+location = /ContentService/data/species/main {
+    include /etc/nginx/common/upstream-proxy.conf;
+    proxy_pass http://content_node;
+}
+
+location = /ContentService/data/species/all {
+    include /etc/nginx/common/upstream-proxy.conf;
+    proxy_pass http://content_node;
+}
+
 # The person page's four lists, which take an id so they cannot be exact
 # matches. `~` is a regex location and those are tried in order before any
 # prefix, so this wins over /ContentService/ below without depending on where in

proxy.conf.js

@@ -63,6 +63,11 @@ module.exports = {
       '/ContentService/data/content/toc',
       '/ContentService/data/content/doi',
       '/ContentService/data/content/contributors',
+      // The species lists. Exact contexts, not a `/data/species` prefix: Java
+      // also serves `/data/species/{taxId}` shapes that node does not, and a
+      // prefix would claim them and 404 what currently works.
+      '/ContentService/data/species/main',
+      '/ContentService/data/species/all',
     ].map((context) => [
       context,
       {

tools/content-node/graph.mjs

@@ -80,6 +80,42 @@ function connection() {
  * of a 64-bit value and a trap when the answer is compared against JSON from
  * another implementation: 74160 must serialise as 74160, not as an object.
  */
+/**
+ * **Match by an indexed property, never by label alone.**
+ *
+ * This instance has **no token (LOOKUP) index** -- `SHOW INDEXES` reports 36
+ * indexes and none of them is one -- so `MATCH (n:Label)` cannot use a label
+ * index and the planner falls back to scanning every node and filtering.
+ * Measured on `MATCH (n:DBInfo) RETURN n.releaseNumber`, which matches exactly
+ * one node:
+ *
+ *     EXPLAIN ->  AllNodesScan  +  Filter n:DBInfo
+ *     nodes in the database      2,958,129
+ *     time                       ~700ms
+ *     the same read via an indexed property (DatabaseObject.dbId)   29ms
+ *
+ * So the cost has nothing to do with how many nodes match or how many rows come
+ * back: one node and four hundred cost the same, because both scan three
+ * million. That is also why Java's own `/data/species/main` takes 682ms.
+ *
+ * Two consequences for anyone porting an endpoint:
+ *
+ * 1. Prefer a property that is indexed, and match it **under the label the
+ *    index is on** -- `dbId` is on `DatabaseObject`, and Neo4j treats `Person`
+ *    as an unrelated label, so `(p:Person) WHERE p.dbId = …` scans while
+ *    `(n:DatabaseObject) WHERE n.dbId = …` does not.
+ * 2. When an endpoint genuinely needs every node of a label -- the species
+ *    lists do, and no property can express that -- there is no query that
+ *    avoids the scan. Wrap it in `cached()` so the scan is paid once at
+ *    startup rather than on a request path.
+ *
+ * The real fix is one statement from whoever owns the database:
+ *
+ *     CREATE LOOKUP INDEX node_labels FOR (n) ON EACH labels(n)
+ *
+ * It would make every label match fast for the Java service too. It is a schema
+ * write on a shared instance, so it is not taken here.
+ */
 export async function read(cypher, parameters = {}) {
   const session = connection().session({
     database: DATABASE,

tools/content-node/service.mjs

@@ -40,19 +40,41 @@ export const endpoints = [
     // still caught a wrong guess. The property is `releaseNumber`; `version` does
     // not exist on DBInfo and returned null, which the diff reported against
     // Java's 97 before anything could be switched on.
-    handler: async () => {
+    /**
+     * Cached, like every other list here, and for a sharper reason: measured
+     * against Java, this answered in 286ms where Java took 1.1ms, because Java
+     * holds the value and this went to the graph on every request. The site
+     * asks for the release on every page load -- it is what keys the bucket
+     * paths for diagrams, figures and icons -- so that was the most-called
+     * endpoint of the set and the slowest.
+     *
+     * Safe for the same reason Java's is: the release number changes when the
+     * database is replaced, and that restarts the service.
+     *
+     * Not fixable with an index on this instance, and the reason is worth
+     * knowing before porting anything else: there is no token index here, so
+     * `MATCH (n:DBInfo)` plans as `AllNodesScan + Filter` and reads all
+     * 2,958,129 nodes to find one. ~700ms, and the same for any label-only
+     * match -- which is why Java's `/data/species/main` takes 682ms too. See
+     * the note on `read` in graph.mjs.
+     *
+     * Caching is the fix available from this side. The service warms every
+     * `cached` handler at startup, so the scan is paid once, off the request
+     * path, and never by a reader.
+     */
+    handler: cached('database/version', async () => {
       const [row] = await read('MATCH (n:DBInfo) RETURN n.releaseNumber AS version LIMIT 1');
       if (!row) return { status: 404, body: 'no DBInfo node' };
       return { status: 200, body: String(row.version), type: 'text/plain;charset=UTF-8' };
-    },
+    }),
   },
   {
     path: '/ContentService/data/database/name',
-    handler: async () => {
+    handler: cached('database/name', async () => {
       const [row] = await read('MATCH (n:DBInfo) RETURN n.name AS name LIMIT 1');
       if (!row) return { status: 404, body: 'no DBInfo node' };
       return { status: 200, body: String(row.name), type: 'text/plain;charset=UTF-8' };
-    },
+    }),
   },
   {
     path: '/ContentService/data/pathways/top/{id}',
@@ -503,6 +525,57 @@ export const endpoints = [
       differs: [/: java normalised by the endpoint's own rule before comparing$/],
     }))
   ),
+  ...[
+    {
+      suffix: 'main',
+      /**
+       * A species Reactome has curated pathways for, which is the list every
+       * species selector on the site offers.
+       *
+       * "Main" is not a property on the node -- all 96 Species carry exactly the
+       * same six -- it is a relationship: 16 of them are the target of at least
+       * one TopLevelPathway's `species`, and 16 is what Java returns. Checked
+       * against the graph rather than inferred from the count matching, because
+       * two numbers agreeing is not a rule.
+       */
+      match: '(s:Species)<-[:species]-(:TopLevelPathway)',
+    },
+    { suffix: 'all', match: '(s:Species)' },
+  ].map(({ suffix, match }) => ({
+    path: `/ContentService/data/species/${suffix}`,
+    /**
+     * The species lists, which differ in their order as well as their contents.
+     *
+     * `main` puts Homo sapiens first and sorts the rest by name; `all` is plain
+     * alphabetical, human included, in its place. Both verified against the live
+     * responses -- and the difference is the reason they are written as two rows
+     * of one generator rather than one handler taking a flag, because a shared
+     * sort would have quietly given `all` a pinned human or `main` an unpinned
+     * one, and the page would still have rendered.
+     *
+     * Reactome pins human because it is the reference species everything else is
+     * inferred from, so a selector that buries it between Gallus and Mus is
+     * asking every reader to hunt for the common case.
+     */
+    handler: cached(`species/${suffix}`, async () => {
+      const rows = await read(
+        `MATCH ${match}
+         WITH DISTINCT s
+         RETURN properties(s) AS species
+         ORDER BY CASE WHEN s.displayName = 'Homo sapiens' THEN 0 ELSE 1 END, s.displayName`
+      );
+      const body = rows.map(({ species }) => ({
+        ...only(species, ['dbId', 'displayName', 'name', 'taxId', 'abbreviation']),
+        className: species.schemaClass,
+        schemaClass: species.schemaClass,
+      }));
+      // `all` is alphabetical throughout, so the pin the query applies for
+      // `main` is undone here rather than by a second query. One ORDER BY that
+      // both share, and one line saying which of them does not want it.
+      if (suffix === 'all') body.sort((a, b) => (a.displayName < b.displayName ? -1 : 1));
+      return { status: 200, body, type: 'application/json' };
+    }),
+  })),
 ];
 
 /**
@@ -523,8 +596,19 @@ export const endpoints = [
  * again. A slow start costs one slow request instead of an empty page nobody
  * connects to a restart hours earlier.
  *
- * Nothing refreshes it while the process lives, which matches Java: a release
- * restarts the service, and these lists only change with the graph.
+ * Held for the life of the process, which makes restarting this service part of
+ * updating the database rather than an optimisation detail. That is the agreed
+ * rule, and it is the rule rather than an inference: the previous version of
+ * this comment asserted that "a release restarts the service", which is true of
+ * Java -- its WAR is redeployed -- and was not true of this, a container with
+ * `restart: unless-stopped` that nothing restarted.
+ *
+ * What it costs to get wrong, so the rule is worth keeping: the release number
+ * keys the bucket paths for diagrams, figures and icons, so a service holding
+ * the previous release's number sends every one of those requests to the wrong
+ * prefix, and keeps doing it silently. `/health` reports the release this
+ * process is holding for exactly that reason -- a missed restart is then one
+ * curl away from being obvious instead of invisible.
  */
 function cached(name, build) {
   let ready;
@@ -704,11 +788,32 @@ export function app() {
   const server = express();
   server.disable('x-powered-by');
 
-  server.get('/health', (_request, response) => {
+  server.get('/health', async (_request, response) => {
+    // The release this process is holding, which is the one thing here that can
+    // be wrong without anything looking wrong. Caches live for the life of the
+    // process by agreement -- updating the database includes restarting this --
+    // and a missed restart shows up as diagram, figure and icon URLs pointing at
+    // the previous release's bucket prefix, silently. Reporting it makes that
+    // one curl away from obvious.
+    //
+    // Read through the same cache the endpoint uses, so this reports what is
+    // being *served* rather than what the graph currently says. A health check
+    // that went straight to the database would answer correctly while every
+    // other response was stale, which is the opposite of useful.
+    let release = null;
+    try {
+      const version = endpoints.find((e) => e.path.endsWith('/data/database/version'));
+      const answer = await version?.handler({ params: {}, query: {} });
+      release = answer?.body ?? null;
+    } catch {
+      // A health check that fails because the graph is down is reporting the
+      // wrong thing: the process is up, and that is what this answers.
+    }
     response.json({
       ok: true,
       ...buildId(),
       graph: configured(),
+      release,
       endpoints: endpoints.map((e) => e.path),
     });
   });

tools/content-node/service.spec.mjs

@@ -102,3 +102,51 @@ describe("the person lists' declared difference", () => {
     expect(person.differs?.length, 'the normalisation is declared').toBeGreaterThan(0);
   });
 });
+
+describe('the species lists', () => {
+  const paths = endpoints.map((e) => e.path);
+
+  it('serves both, on the paths Java uses', () => {
+    expect(paths).toContain('/ContentService/data/species/main');
+    expect(paths).toContain('/ContentService/data/species/all');
+  });
+
+  it('serves them as two entries rather than one taking a flag', () => {
+    // The two differ in order as well as contents -- `main` pins Homo sapiens
+    // first, `all` is plain alphabetical including human in its place -- and a
+    // single handler with a boolean is how one of those silently acquires the
+    // other's sort. Both orders are proven against Java by `diff.mjs`, which
+    // needs a database and so cannot run here; what this can hold is that they
+    // stayed separate.
+    const species = endpoints.filter((e) => e.path.includes('/data/species/'));
+    expect(species).toHaveLength(2);
+    expect(species[0].handler).not.toBe(species[1].handler);
+  });
+});
+
+describe('what /health says about staleness', () => {
+  /**
+   * Caches live for the life of the process by agreement: updating the database
+   * includes restarting this service. The previous version of that reasoning
+   * was an inference — "a release restarts the service", true of Java's WAR and
+   * not of a container with `restart: unless-stopped` — so the rule is now the
+   * rule, and this is what makes a missed restart visible.
+   *
+   * It matters because the release number keys the bucket paths for diagrams,
+   * figures and icons: a process holding the previous one sends all of those to
+   * the wrong prefix, and nothing else about the response looks wrong.
+   */
+  it('reports the release it is serving', async () => {
+    const health = await (await fetch(`${base}/health`)).json();
+    expect(health).toHaveProperty('release');
+  });
+
+  it('still answers when the graph cannot be reached', async () => {
+    // No credentials in this suite, so the version handler throws. A health
+    // check that failed on that would be reporting the database's health under
+    // the name of the process's.
+    const response = await fetch(`${base}/health`);
+    expect(response.status).toBe(200);
+    expect((await response.json()).ok).toBe(true);
+  });
+});