From b58d96e3ae0b9abb3d32cbc0901ad0ed33955aaa Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Sat, 17 Oct 2026 14:59:49 +0000 Subject: [PATCH 1/2] Port POST /data/query/ids and /data/query/ids/map to content-node Reactome's batch lookup: up to twenty objects by id in a text/plain body. - Ids read as Java reads them: split on , ; newline and tab, trimmed; past twenty, the first twenty in a HashSet (orthologyIds). - Each id is findById(id, OUTGOING), the lookup /query/{id} does, now one function both use (findObject), asked one at a time. - /ids writes graph-core's HashSet of DatabaseObjects in its order, by DatabaseObject.hashCode (dbId, stId, displayName, 31 apart; Long.hashCode past an int); /ids/map writes a HashMap of the ids as sent. Both orders checked against the JDK's own (11). Each object is its own lookup, so one reached by two ids is written in full twice, as Jackson's per-instance identity does. - Java's errors: 415 for a body that is not text/plain, 422 naming the method for a missing one, a 404 naming the ids as a Java list, a 500 for a dbId past a long; a GET of /ids/map answers as Java's attribute route does (no object called "ids"). - Routed in nginx and the dev proxy, matrix parameter and trailing slash included; the ids exclusion #498 added comes out. Byte-identical to Java live over ten requests -- mixed id kinds and separators, duplicates, missing ids, over twenty ids -- bar the nested views' known key order, declared as /query/{id} declares it. Spec 006 T071. Co-Authored-By: Claude Opus 5.5 --- deploy/nginx/common/routes-core.conf | 9 +- .../src/scripts/proxy-order.spec.ts | 9 +- .../src/scripts/serve-prod.spec.ts | 4 +- proxy.conf.js | 5 +- tools/content-node/query-ids.spec.mjs | 144 +++++++++++++++++ tools/content-node/service.mjs | 147 ++++++++++++++++-- 6 files changed, 298 insertions(+), 20 deletions(-) create mode 100644 tools/content-node/query-ids.spec.mjs diff --git a/deploy/nginx/common/routes-core.conf b/deploy/nginx/common/routes-core.conf index 1e1fe6fa..adf3c084 100644 --- a/deploy/nginx/common/routes-core.conf +++ b/deploy/nginx/common/routes-core.conf @@ -297,10 +297,13 @@ location ~ ^/ContentService/data/pathways/top/[^/]+/?$ { # other endpoints. Byte-identical to Java over a corpus of 20,000 objects of # every class, bar the order of a few getter-only properties in the nested # views, which Java itself does not keep from one start to the next. -# `ids` is not an id: it is Java's POST /query/ids, a batch node does not -# serve, and a location matches whatever the method. +# Including POST /query/ids, a batch of objects by id, and its map form +# below: node answers both, and a GET of either as Java does. +location ~ ^/ContentService/data/query/[^/]+/?$ { + include /etc/nginx/common/content-node-proxy.conf; +} # Quoted: unquoted, nginx reads the ';' as the end of the directive. -location ~ "^/ContentService/data/query/(?!ids(;[^/]*)?/?$)[^/]+/?$" { +location ~ "^/ContentService/data/query/ids(;[^/]*)?/map/?$" { include /etc/nginx/common/content-node-proxy.conf; } diff --git a/projects/website-angular/src/scripts/proxy-order.spec.ts b/projects/website-angular/src/scripts/proxy-order.spec.ts index c7b6dd8a..69d82087 100644 --- a/projects/website-angular/src/scripts/proxy-order.spec.ts +++ b/projects/website-angular/src/scripts/proxy-order.spec.ts @@ -237,6 +237,10 @@ describe('the dev server proxy', () => { '/ContentService/data/instance/R-HSA-141433/referrers', // The PSICQUIC services the Pathway Browser can draw interactors from. '/ContentService/interactors/psicquic/resources', + // A batch of objects by id (POST), with a matrix parameter too, and as a map. + '/ContentService/data/query/ids', + '/ContentService/data/query/ids;x=1', + '/ContentService/data/query/ids/map', // Spring matches a trailing slash, and so must the route once Java is gone. '/ContentService/data/instance/R-HSA-141433/referrers/', ]) { @@ -256,11 +260,6 @@ describe('the dev server proxy', () => { '/ContentService/data/query/abridged/stId', // Java matches attribute names in any case; only the site's spelling is routed. '/ContentService/data/query/70486/NORMALPATHWAY', - // Java's POST /query/ids (a batch of objects), which node does not serve. - '/ContentService/data/query/ids', - '/ContentService/data/query/ids/', - // With a matrix parameter, which Spring drops before matching. - '/ContentService/data/query/ids;x=1', '/ContentService/data/query/abridged/70486', // The search endpoints node does not serve yet. '/ContentService/search/query/x?query=apoptosis', diff --git a/projects/website-angular/src/scripts/serve-prod.spec.ts b/projects/website-angular/src/scripts/serve-prod.spec.ts index ddfa0dfd..17093f08 100644 --- a/projects/website-angular/src/scripts/serve-prod.spec.ts +++ b/projects/website-angular/src/scripts/serve-prod.spec.ts @@ -317,7 +317,9 @@ describe('serve-prod and the API roots', () => { ['/ContentService/data/query/R-HSA-70486?view=nested', 'content-node'], ['/ContentService/data/query/enhanced/R-HSA-70486', 'content-node'], ['/ContentService/data/query/enhanced/v2/R-HSA-70486', 'content-node'], - ['/ContentService/data/query/ids', 'backend'], + // The batch lookup is node's too (spec 006 T071). + ['/ContentService/data/query/ids', 'content-node'], + ['/ContentService/data/query/ids/map', 'content-node'], ]) { it(`sends ${url} to ${to}`, async () => { expect(await (await fetch(base + url)).text()).toBe(`${to} ${url}`); diff --git a/proxy.conf.js b/proxy.conf.js index 3ef9a673..cea95221 100644 --- a/proxy.conf.js +++ b/proxy.conf.js @@ -154,8 +154,9 @@ module.exports = { // One object with everything it points to, in any of Java's views // (?view=, ?includeRef=). Only the plain path: /query/{id}/... and Java's // /query/enhanced|abridged/{id} are other endpoints. - // `ids` is Java's POST /query/ids, a batch, which node does not serve. - '^/ContentService/data/query/(?!ids(;[^/?]*)?/?(\\?|$))[^/?]+/?(\\?|$)': contentNode, + // Including POST /query/ids, a batch of objects by id, and its map form. + '^/ContentService/data/query/[^/?]+/?(\\?|$)': contentNode, + '^/ContentService/data/query/ids(;[^/?]*)?/map/?(\\?|$)': contentNode, // An object's stable id by any id Java accepts (13 bytes against 12KB for // the whole object), and a disease pathway's normal pathway. Only these two // attributes of Java's /data/query/{id}/{attributeName} are ported, so the diff --git a/tools/content-node/query-ids.spec.mjs b/tools/content-node/query-ids.spec.mjs new file mode 100644 index 00000000..766404de --- /dev/null +++ b/tools/content-node/query-ids.spec.mjs @@ -0,0 +1,144 @@ +// @vitest-environment node +// +// POST /data/query/ids and /data/query/ids/map -- a batch of objects by id -- +// with a stand-in graph that finds nothing, for Java's errors, and the two +// orders Java writes in: a HashSet of DatabaseObjects and a HashMap of ids. +// The expected orders are the JDK's own (11, the version the service runs), +// computed with graph-core's DatabaseObject.hashCode -- dbId, then stId, +// then displayName, 31 apart. Java's answers measured 17 Oct 2026. +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; + +vi.mock('./graph.mjs', () => ({ + read: async () => [], + configured: () => true, + close: async () => {}, + envSetting: () => '', +})); + +const { app, databaseObjectSet, idKeyedMap } = await import('./service.mjs'); + +const object = (dbId, stId, displayName) => ({ properties: { dbId, stId, displayName } }); + +describe('the orders Java writes', () => { + it("a HashSet of DatabaseObjects, by graph-core's hashCode", () => { + const objects = [ + object(109581, 'R-HSA-109581', 'Apoptosis'), + object(1640170, 'R-HSA-1640170', 'Cell Cycle'), + object(199420, 'R-HSA-199420', 'PTEN [cytosol]'), + object(69620, 'R-HSA-69620', 'Cell Cycle Checkpoints'), + object(29358, 'R-ALL-29358', 'ATP [cytosol]'), + object(5672965, 'R-HSA-5672965', 'RAS GEFs'), + // Past an int, and with a field missing: Long.hashCode, and 0 for null. + object(9999999999, 'R-HSA-9', null), + object(1, null, 'x'), + ]; + expect(databaseObjectSet(objects).map((o) => o.properties.dbId)).toEqual([ + 1640170, 1, 109581, 29358, 5672965, 9999999999, 199420, 69620, + ]); + }); + + it('keeps one of two equal objects, as a set does', () => { + const a = object(109581, 'R-HSA-109581', 'Apoptosis'); + expect(databaseObjectSet([a, object(109581, 'R-HSA-109581', 'Apoptosis')])).toHaveLength(1); + }); + + it('a HashMap of the ids as sent', () => { + const entries = ['R-HSA-109581', '199420', 'REACT_578', 'R-HSA-1640170', '69620', 'xyz'].map( + (id) => [id, id] + ); + expect(idKeyedMap(entries).map(([id]) => id)).toEqual([ + '69620', + 'REACT_578', + 'R-HSA-1640170', + 'R-HSA-109581', + 'xyz', + '199420', + ]); + }); +}); + +let base; +let server; +beforeAll(async () => { + await new Promise((resolve) => { + server = app().listen(0, '127.0.0.1', resolve); + }); + base = `http://127.0.0.1:${server.address().port}`; +}); +afterAll(() => new Promise((resolve) => server.close(resolve))); + +const post = (path, body, type = 'text/plain') => + fetch(`${base}/ContentService/data/query/${path}`, { + method: 'POST', + headers: type ? { 'content-type': type } : {}, + body, + }); + +describe("Java's errors", () => { + it('refuses a body that is not text/plain, before anything else', async () => { + const response = await post('ids', 'x', 'application/json'); + expect([response.status, (await response.json()).messages]).toEqual([ + 415, + ["Content type 'application/json' not supported"], + ]); + }); + + for (const [path, signature] of [ + [ + 'ids', + 'public java.util.Collection org.reactome.server.service.controller.graph.QueryObjectController.findByIds(java.lang.String)', + ], + [ + 'ids/map', + 'public java.util.Map org.reactome.server.service.controller.graph.QueryObjectController.findByIdsMap(java.lang.String)', + ], + ]) + it(`answers a missing body at ${path} with Spring's 422`, async () => { + const response = await post(path, undefined); + expect([response.status, (await response.json()).messages]).toEqual([ + 422, + [`Required request body is missing: ${signature}`], + ]); + }); + + it('names the ids it did not find, as a Java list', async () => { + for (const path of ['ids', 'ids/map']) { + const response = await post(path, 'xyz,abc'); + expect([path, response.status, (await response.json()).messages]).toEqual([ + path, + 404, + ['Ids: [xyz, abc] have not been found in the System'], + ]); + } + }); + + it("is Java's 500 for a dbId past a long", async () => { + const response = await post('ids', '99999999999999999999'); + expect([response.status, (await response.json()).messages]).toEqual([ + 500, + ['For input string: "99999999999999999999"'], + ]); + }); + + it('answers a GET as the attribute route of Java does: no object called "ids"', async () => { + for (const path of ['ids/map', 'ids/']) { + const response = await fetch(`${base}/ContentService/data/query/${path}`); + expect([path, response.status, (await response.json()).messages]).toEqual([ + path, + 404, + ['Id: ids has not been found in the System'], + ]); + } + }); + + it('takes the trailing slash and a matrix parameter, as Spring does', async () => { + for (const path of ['ids/', 'ids;x=1', 'ids/map/']) { + const response = await post(path, 'xyz'); + expect([path, response.status, (await response.json()).messages]).toEqual([ + path, + 404, + ['Ids: [xyz] have not been found in the System'], + ]); + } + }); +}); diff --git a/tools/content-node/service.mjs b/tools/content-node/service.mjs index 7539e31b..4418e184 100644 --- a/tools/content-node/service.mjs +++ b/tools/content-node/service.mjs @@ -844,21 +844,88 @@ export const endpoints = [ */ handler: async (request) => { const raw = pathId(rawSegment(request, 4)); - const { where, params } = resolveId(raw); - const [found] = await read(rootLabelsQuery(where), params); - if (!found) return notFound(request, `Id: ${raw} has not been found in the System`); - const [row] = await read(findByIdQuery(where), { - ...params, - ...findByIdParameters(found.labels), - }); - if (!row) return notFound(request, `Id: ${raw} has not been found in the System`); + const object = await findObject(raw); + if (!object) return notFound(request, `Id: ${raw} has not been found in the System`); return { status: 200, - serialised: writeDomain(mapResult(row), ...writerFor(request)), + serialised: writeDomain(object, ...writerFor(request)), type: 'application/json', }; }, }, + ...[ + { + path: '/ContentService/data/query/ids', + signature: + 'public java.util.Collection org.reactome.server.service.controller.graph.QueryObjectController.findByIds(java.lang.String)', + // QueryObjectController.findByIds: graph-core's findByIds, each id's + // findById gathered in a HashSet -- written in that set's order. + answer: (found) => databaseObjectSet(found.map(([, object]) => object)), + }, + { + path: '/ContentService/data/query/ids/map', + signature: + 'public java.util.Map org.reactome.server.service.controller.graph.QueryObjectController.findByIdsMap(java.lang.String)', + // QueryObjectController.findByIdsMap: each id as sent to its object, in + // a HashMap -- written in that map's order. Each object is its own + // lookup, so one reached by two ids is written in full twice, as + // Jackson's identity is per instance. + answer: (found) => new JsonMap(idKeyedMap(found)), + }, + ].map(({ path, signature, answer }) => ({ + path, + method: 'post', + sampleBody: 'R-HSA-1640170, R-HSA-109581, 199420\nREACT_578;xyz', + differs: [ + new RegExp(`^${path}: \\.url: `), + // The nested views' key order, which Java does not keep from one start to + // the next: see /query/{id}. + /^[^:]*(?=[^:]*includeRef=true)(?=[^:]*view=nested)[^:]*: \S*\.@(id|ref): /, + ], + /** + * A batch of objects, by the ids in a text/plain body: split on , ; + * newline and tab, trimmed; past twenty, the first twenty in a HashSet + * (orthologyIds, which reads them as Java does). Each is findById, as + * /query/{id} finds one, one at a time on the database the site shares. + * None found is a 404 naming the ids as a Java list; a dbId past a long + * anywhere is Java's 500 for the whole request. + */ + handler: async (request) => { + const contentType = request.get?.('content-type'); + const problem = plainTextProblem(contentType); + if (problem) return javaError(request, 415, 'UNSUPPORTED_MEDIA_TYPE', problem); + if (request.tooLarge) + return javaError(request, 413, 'PAYLOAD_TOO_LARGE', 'More than 10MB of ids'); + if (!Buffer.isBuffer(request.body) || request.body.length === 0) + return javaError( + request, + 422, + 'UNPROCESSABLE_ENTITY', + `Required request body is missing: ${signature}` + ); + const ids = orthologyIds(plainText(request.body, contentType)); + const refused = longPastRange(ids); + if (refused) return javaError(request, 500, 'INTERNAL_SERVER_ERROR', refused); + const found = []; + for (const id of ids) { + const object = await findObject(id); + if (object) found.push([id, object]); + } + if (found.length === 0) + return notFound(request, `Ids: [${ids.join(', ')}] have not been found in the System`); + return { + status: 200, + serialised: writeDomain(answer(found), ...writerFor(request)), + type: 'application/json', + }; + }, + })), + { + path: '/ContentService/data/query/ids/map', + // A GET here is Java's /query/{id}/{attributeName}, for an object called + // "ids" -- which nothing is. + handler: async (request) => notFound(request, 'Id: ids has not been found in the System'), + }, ...[ { kind: 'entity', @@ -3272,6 +3339,68 @@ function utf32(body, order) { return at < body.length ? `${text}\ufffd` : text; } +/** + * One object with everything it points to, as graph-core's + * findById(id, OUTGOING) gives it; undefined when there is none. + */ +async function findObject(raw) { + const { where, params } = resolveId(raw); + if (where === 'false') return undefined; + const [found] = await read(rootLabelsQuery(where), params); + if (!found) return undefined; + const [row] = await read(findByIdQuery(where), { + ...params, + ...findByIdParameters(found.labels), + }); + return row ? mapResult(row) : undefined; +} + +/** Long.hashCode, as an int: the value's two halves XORed. */ +function longHash(value) { + const big = BigInt.asUintN(64, BigInt(value)); + return Number(BigInt.asIntN(32, big ^ (big >> 32n))); +} + +/** + * graph-core's DatabaseObject.hashCode: dbId, then stId, then displayName, + * 31 apart, 0 for a missing one. Equal objects (the same three) are one. + */ +function databaseObjectHash({ dbId, stId, displayName }) { + let result = dbId == null ? 0 : longHash(dbId); + result = (Math.imul(31, result) + (stId == null ? 0 : JavaHashMap.stringHash(stId))) | 0; + return ( + (Math.imul(31, result) + (displayName == null ? 0 : JavaHashMap.stringHash(displayName))) | 0 + ); +} + +/** A java.util.HashSet of DatabaseObjects, in its iteration order. */ +export function databaseObjectSet(objects) { + const byKey = new Map(); + const keyOf = (object) => { + const { dbId, stId, displayName } = object.properties; + return JSON.stringify([dbId ?? null, stId ?? null, displayName ?? null]); + }; + const set = new JavaHashMap({ + hash: (key) => { + const [dbId, stId, displayName] = JSON.parse(key); + return databaseObjectHash({ dbId, stId, displayName }); + }, + }); + for (const object of objects) { + const key = keyOf(object); + if (!byKey.has(key)) byKey.set(key, object); + set.set(key, true); + } + return [...set.keys()].map((key) => byKey.get(key)); +} + +/** A java.util.HashMap of these entries, put in this order, as its iteration order. */ +export function idKeyedMap(entries) { + const map = new JavaHashMap(); + for (const [key, value] of entries) map.set(key, value); + return [...map.entries()]; +} + /** * A java.util.HashMap's iteration order, for answers Java writes from one -- * its put, resize and tree bins ported from the JDK the service runs on From 7c28a80372fb21829ff3d3d1e95bd163a2db56d0 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Sat, 17 Oct 2026 15:20:12 +0000 Subject: [PATCH 2/2] Review of #509: an id of dots fails the batch, as Java's trimId does graph-core's trimId splits an id on '.' and takes the first part; "." (or "..") splits to nothing, and Java answers the whole batch with a 500 ("Index 0 out of bounds for length 0"). Node skipped the id and answered 200 with the rest -- a fail-open. Java looks ids up one at a time, so the first bad id decides between that and a dbId past a long: both are now one in-order pass (findByIdFailure). Byte-identical to Java live on both endpoints, each order of the two, and "..x" (which Java reads as an empty id, not a failure). Spec 006 T071 ticked. Co-Authored-By: Claude Opus 5.5 --- specs/006-content-service-to-node/tasks.md | 2 +- tools/content-node/query-ids.spec.mjs | 19 +++++++++++++++++++ tools/content-node/service.mjs | 19 ++++++++++++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/specs/006-content-service-to-node/tasks.md b/specs/006-content-service-to-node/tasks.md index ff354e9d..79901fae 100644 --- a/specs/006-content-service-to-node/tasks.md +++ b/specs/006-content-service-to-node/tasks.md @@ -74,7 +74,7 @@ Each port follows the eight steps in [plan.md](plan.md#how-one-endpoint-is-porte ## Phase 7: Remaining graph and index operations → node - [ ] T070 `GET /data/query/{id}/{attributeName}` for the attributes not yet ported (only `stId` and `normalPathway` are) -- [ ] T071 `POST /data/query/ids`, `POST /data/query/ids/map` +- [x] T071 `POST /data/query/ids`, `POST /data/query/ids/map` — #509 - [ ] T072 `GET /data/person/{id}/{attributeName}` - [ ] T073 `GET /data/people/name/{name}`, `GET /data/people/name/{name}/exact` - [ ] T074 `GET /data/schema/{className}`, `GET /data/schema/{className}/reference` diff --git a/tools/content-node/query-ids.spec.mjs b/tools/content-node/query-ids.spec.mjs index 766404de..ff6e29a1 100644 --- a/tools/content-node/query-ids.spec.mjs +++ b/tools/content-node/query-ids.spec.mjs @@ -112,6 +112,25 @@ describe("Java's errors", () => { } }); + it("is Java's 500 for an id of dots, whatever else the body holds", async () => { + // graph-core's trimId splits on '.', and "." splits to nothing. + for (const body of ['R-HSA-109581,.', '..', 'x;.']) { + const response = await post('ids', body); + expect([body, response.status, (await response.json()).messages]).toEqual([ + body, + 500, + ['Index 0 out of bounds for length 0'], + ]); + } + }); + + it('fails on the first bad id, in order, as Java looks them up one at a time', async () => { + const dots = await post('ids', '.,99999999999999999999'); + expect((await dots.json()).messages).toEqual(['Index 0 out of bounds for length 0']); + const long = await post('ids', '99999999999999999999,.'); + expect((await long.json()).messages).toEqual(['For input string: "99999999999999999999"']); + }); + it("is Java's 500 for a dbId past a long", async () => { const response = await post('ids', '99999999999999999999'); expect([response.status, (await response.json()).messages]).toEqual([ diff --git a/tools/content-node/service.mjs b/tools/content-node/service.mjs index 4418e184..9be8c97a 100644 --- a/tools/content-node/service.mjs +++ b/tools/content-node/service.mjs @@ -904,7 +904,7 @@ export const endpoints = [ `Required request body is missing: ${signature}` ); const ids = orthologyIds(plainText(request.body, contentType)); - const refused = longPastRange(ids); + const refused = findByIdFailure(ids); if (refused) return javaError(request, 500, 'INTERNAL_SERVER_ERROR', refused); const found = []; for (const id of ids) { @@ -3161,6 +3161,23 @@ function longPastRange(ids) { return undefined; } +/** + * What graph-core's findById throws on, for ids looked up one at a time, in + * order -- so the first bad id decides: an id of dots (trimId splits it on + * '.' to nothing, an ArrayIndexOutOfBounds) or a dbId past a long. + * Undefined when Java would throw on none. + */ +function findByIdFailure(ids) { + for (const raw of ids) { + const trimmed = javaTrim(raw); + if (!trimmed.includes(':') && /^\.+$/.test(trimmed)) + return 'Index 0 out of bounds for length 0'; + const refused = longPastRange([raw]); + if (refused) return refused; + } + return undefined; +} + /** * The ids in getOrthologies' body: String.split on , ; newline or tab -- * trailing empty strings dropped, as Java's split drops them -- each trimmed;