From 2054a51760f1769316ef062e71de078d22f54261 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Thu, 15 Oct 2026 15:11:55 +0000 Subject: [PATCH 1/2] Port /data/query/enhanced/v2/{id} to node, SummaryEntity included The site's busiest content call: the Pathway Browser's details panel and the website's detail pages fetch every object they show through it (fetchIncomingRelationships, summariseReferenceEntity and includeDisease all true, JSOG, nested-aggregated). - The query is graph-core's findEnhancedObjectById, as written in AdvancedDatabaseObjectRepository. When summarising a reference entity, it covers the object and every physical entity referring to it, with their neighbours (both ways unless needsIncoming says not) and the repository's second-step expansions. UpdateTrackers are excluded, and so is disease data unless includeDisease. - A reference entity, asked to summarise, is written as content-service's SummaryEntity, rebuilt in node (summaryEntity). The constructor's copies by reference type. The physical entities are sorted by modified residues then dbId, in the reference entity's own list too, as Java sorts in place. Each accessor merges through SummaryEntity's getter and PhysicalEntity's setter: a list unioned once per element, the stoichiometric ones expanded and re-aggregated, the species and reference type of the last entity that has one. - SummaryEntity's Jackson layout comes from the deployed classes like everything else. snapshot-domain reads it serialise-only, keeping it out of SDN's mapping context. Its two computed getters are implemented, and the generator refuses to run until they are. - Three primitive booleans with defaults are parsed as Spring parses them (blank is a 400 naming `boolean`). An object found only with disease data, asked for without, is looked up again with it, as in Java. Against Java, 270 objects (reactions, pathways, complexes, EWASs, sets, chemicals, gene products, molecules), spaced out per the load rules: - default view: 270 of 270 byte-identical; - the site's nested JSOG view: 270 of 270 identical as object graphs. The nested view's getter-only key order is the JVM's of the day (see writeDomain). That moves where an object is written in full and so its JSOG number, and those views are compared that way. Node took 5.0s in all, Java 13.1s. Tests failed before the change. Reverting the residue sort, the de-duplication or the species merge each fails one. nginx -t passes. Co-Authored-By: Claude Opus 5.5 --- deploy/nginx/common/routes-core.conf | 15 +- .../src/scripts/proxy-order.spec.ts | 3 +- .../src/scripts/serve-prod.spec.ts | 2 +- proxy.conf.js | 6 +- tools/content-node/domain-snapshot.json | 8 +- tools/content-node/domain.mjs | 207 +++++++++++++++++- tools/content-node/domain.spec.mjs | 84 +++++++ tools/content-node/service.mjs | 79 +++++++ tools/content-node/service.spec.mjs | 19 ++ tools/content-node/snapshot-domain.mjs | 30 ++- .../snapshot-domain/DomainSnapshot.java | 18 +- 11 files changed, 456 insertions(+), 15 deletions(-) diff --git a/deploy/nginx/common/routes-core.conf b/deploy/nginx/common/routes-core.conf index d7e28596..be01c50c 100644 --- a/deploy/nginx/common/routes-core.conf +++ b/deploy/nginx/common/routes-core.conf @@ -74,11 +74,22 @@ location = /ContentService/data/species/all { proxy_pass http://content_node; } +# An object with everything around it, for the Pathway Browser's details panel +# and the website's detail pages: graph-core's findEnhancedObjectById, written +# by the domain serialiser, and a reference entity -- when asked to summarise -- +# as the content service's SummaryEntity, rebuilt in node. Byte-identical to +# Java in the default view; in the nested views (the site's), identical as +# data, the getter-only key order being the JVM's of the day. +location ~ ^/ContentService/data/query/enhanced/v2/[^/]+$ { + include /etc/nginx/common/upstream-proxy.conf; + proxy_pass http://content_node; +} + # An object with its neighbours, their species, and their regulations, # references and markers one step further: graph-core's # findOldEnhancedObjectById, written by the domain serialiser. The entity -# popup, the website's content pages and the IDG panel read it. Not -# /enhanced/v2/{id}, which Java still serves. Byte-identical to Java over +# popup, the website's content pages and the IDG panel read it. Byte-identical +# to Java over # 6,000 objects of every class. location ~ ^/ContentService/data/query/enhanced/[^/]+$ { include /etc/nginx/common/upstream-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 20d71d2d..935144e4 100644 --- a/projects/website-angular/src/scripts/proxy-order.spec.ts +++ b/projects/website-angular/src/scripts/proxy-order.spec.ts @@ -71,6 +71,8 @@ describe('the dev server proxy', () => { } for (const url of [ + '/ContentService/data/query/enhanced/v2/R-HSA-70486', + '/ContentService/data/query/enhanced/v2/uniprot:P01112?summariseReferenceEntity=true&includeRef=true', '/ContentService/data/query/enhanced/R-HSA-70486', '/ContentService/data/query/enhanced/70486?includeRef=true', // Java's /query/enhanced/{id} with the id `normalPathway`, not @@ -193,7 +195,6 @@ describe('the dev server proxy', () => { '/ContentService/data/query/70486/', // Java's POST /query/ids (a batch of objects); node serves no POST. '/ContentService/data/query/ids', - '/ContentService/data/query/enhanced/v2/70486', '/ContentService/data/query/abridged/70486', '/ContentService/data/person/68285/authoredPathwaysAndMore', // Only the ancestors of an event are ported. diff --git a/projects/website-angular/src/scripts/serve-prod.spec.ts b/projects/website-angular/src/scripts/serve-prod.spec.ts index a87bf6e6..ddfa0dfd 100644 --- a/projects/website-angular/src/scripts/serve-prod.spec.ts +++ b/projects/website-angular/src/scripts/serve-prod.spec.ts @@ -316,7 +316,7 @@ describe('serve-prod and the API roots', () => { ['/ContentService/data/query/R-HSA-70486', 'content-node'], ['/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', 'backend'], + ['/ContentService/data/query/enhanced/v2/R-HSA-70486', 'content-node'], ['/ContentService/data/query/ids', 'backend'], ]) { it(`sends ${url} to ${to}`, async () => { diff --git a/proxy.conf.js b/proxy.conf.js index ca4e931b..3968fade 100644 --- a/proxy.conf.js +++ b/proxy.conf.js @@ -102,8 +102,10 @@ module.exports = { }, ]) ), - // An object with its neighbours and some of theirs (the older enhanced - // query). Not /enhanced/v2/, which Java still serves. + // An object with everything around it, for the details panel and the + // website's detail pages; a reference entity as a summary of its entities. + '^/ContentService/data/query/enhanced/v2/[^/?]+(\\?|$)': contentNode, + // An object with its neighbours and some of theirs (the older enhanced query). '^/ContentService/data/query/enhanced/[^/?]+(\\?|$)': contentNode, // An entity or an event and everything below it, for the object tree. '^/ContentService/data/(entity|event)/[^/?]+/in-depth(\\?|$)': contentNode, diff --git a/tools/content-node/domain-snapshot.json b/tools/content-node/domain-snapshot.json index dffc4843..266d2772 100644 --- a/tools/content-node/domain-snapshot.json +++ b/tools/content-node/domain-snapshot.json @@ -255,7 +255,10 @@ ["type","OutputForReactionLikeEvent.getType",7], ["type","PublicationAuthor.getType",7], ["type","RepeatedUnit.getType",7], - ["type","RepeatedUnitForPhysicalEntity.getType",7] + ["type","RepeatedUnitForPhysicalEntity.getType",7], + ["summarisedEntities","summarisedEntities",7], + ["hasModifiedResidue","SummaryEntity.getHasModifiedResidue",1], + ["className","SummaryEntity.getClassName",7] ], "json": { "Affiliation": [0,1,2,3,4,5,6,7,8,9], @@ -366,7 +369,8 @@ "PublicationAuthor": [253,233,234,235], "RepeatedUnit": [254,233,234,235], "RepeatedUnitForPhysicalEntity": [255,233,234,235], - "Default": [221,233,234,235] + "Default": [221,233,234,235], + "SummaryEntity": [0,1,2,3,4,5,11,19,7,22,50,23,80,81,82,24,26,27,30,83,84,34,85,35,86,87,88,89,90,91,92,38,39,41,93,94,47,48,256,201,10,12,200,14,16,137,185,202,203,204,205,206,207,40,211,212,213,215,216,154,127,153,155,257,258,158,71,101,102,104,103,105,106,107,9] }, "fields": [ ["referenceSequence","referenceSequence","OUTGOING","ReferenceSequence","",null], diff --git a/tools/content-node/domain.mjs b/tools/content-node/domain.mjs index 7eba5514..63ade5de 100644 --- a/tools/content-node/domain.mjs +++ b/tools/content-node/domain.mjs @@ -90,7 +90,8 @@ function composedOf(entity) { throw new Error(`domain: ${entity.schemaClass} is not a CompositionAggregator`); } -const isA = (schemaClass, target) => supertypes(schemaClass).has(target); +/** Whether a class is, or descends from, another (Class.isAssignableFrom). */ +export const isA = (schemaClass, target) => supertypes(schemaClass).has(target); /** * What SDN's mapping makes of one query result: the root and every node @@ -345,6 +346,15 @@ const COMPUTED = { 'Has.getStoichiometry': (relation) => relation.stoichiometry, 'Has.getElement': (relation) => relation.element, 'CompositionAggregator.getComposedOf': composedOf, + // The content service's SummaryEntity (summaryEntity, below). + 'SummaryEntity.getHasModifiedResidue': expanding('hasModifiedResidue'), + // Otherwise super.getClassName(): DatabaseObject's, the class's own name. + 'SummaryEntity.getClassName': (entity) => + ({ + ReferenceGeneProduct: 'Protein', + ReferenceDNASequence: 'DNA Sequence', + ReferenceRNASequence: 'RNA Sequence', + })[entity.fields.referenceType] ?? 'SummaryEntity', }; /** The computed getters, by name, for snapshot-domain.mjs to check against graph-core. */ @@ -592,6 +602,201 @@ export function oldEnhancedQuery({ idType, outgoingOnly }) { [r IN rs | ${rel('r')}] AS relationships`; } +/** + * graph-core's findEnhancedObjectById (/query/enhanced/v2), as its + * AdvancedDatabaseObjectRepository writes it: the object -- or, summarising + * a reference entity, the object and every physical entity referring to it -- + * with everything related to each (outgoing only, or both ways), their + * species, reference entities, authors, compartments, regulations and the + * rest one step further. UpdateTrackers are left out; so, unless + * `includeDisease`, is anything in disease. Java's text, with its RETURN + * taken apart as findByIdQuery's is. + */ +export function enhancedQuery({ idType, summarise, includeDisease, outgoingOnly }) { + const node = (n) => `{id: id(${n}), labels: labels(${n}), props: properties(${n})}`; + const rel = (r) => + `{id: id(${r}), type: type(${r}), start: id(startNode(${r})), end: id(endNode(${r})), props: properties(${r})}`; + const expand = !summarise + ? 'WITH root, root AS n ' + : `OPTIONAL MATCH (root:ReferenceEntity)<-[:referenceEntity]-(pe:PhysicalEntity${includeDisease ? '' : ' {isInDisease: false}'}) + WITH collect(pe) + root AS ns, root + UNWIND ns AS n `; + const filter = includeDisease + ? 'WHERE NOT m:UpdateTracker ' + : 'WHERE (m.isInDisease IS NULL OR m.isInDisease = false) AND NOT m:UpdateTracker '; + return `MATCH (root:DatabaseObject) WHERE root.${idType} = ${idType === 'dbId' ? 'toInteger($id)' : '$id'} + ${expand} + OPTIONAL MATCH (n)-[r1]-${outgoingOnly ? '>' : ''}(m:DatabaseObject) + ${filter} + OPTIONAL MATCH (m)-[r2:species]->(s) + OPTIONAL MATCH (m)-[r9:referenceEntity]->(rf:ReferenceEntity) + OPTIONAL MATCH (m:Publication)<-[r5:author]-(p2:Person) + OPTIONAL MATCH (m:InstanceEdit)<-[r6:author]-(p3:Person) + OPTIONAL MATCH (m:Event)-[r7:compartment]->(c:Compartment) + OPTIONAL MATCH (m)-[r3:regulator|regulatedBy|physicalEntity|crossReference|referenceGene| + referenceTranscript|literatureReference|marker|regulation| + catalystActivity|activeUnit|activity|psiMod|modification| + goBiologicalProcess]-(o) + OPTIONAL MATCH (o:Publication)<-[r4:author]-(p:Person) + OPTIONAL MATCH (o)-[r8:species]->(s1:Species) + WITH root, + collect(DISTINCT n) AS ns, collect(DISTINCT m) AS ms, collect(DISTINCT s) AS ss, + collect(DISTINCT o) AS os, collect(DISTINCT p) AS ps, collect(DISTINCT p2) AS ps2, + collect(DISTINCT p3) AS ps3, collect(DISTINCT c) AS cs, collect(DISTINCT s1) AS s1s, + collect(DISTINCT rf) AS rfs, + collect(DISTINCT r1) AS r1s, collect(DISTINCT r2) AS r2s, collect(DISTINCT r3) AS r3s, + collect(DISTINCT r4) AS r4s, collect(DISTINCT r5) AS r5s, collect(DISTINCT r6) AS r6s, + collect(DISTINCT r7) AS r7s, collect(DISTINCT r8) AS r8s, collect(DISTINCT r9) AS r9s + RETURN ${node('root')} AS root, + [x IN ns + ms + ss + os + ps + ps2 + ps3 + cs + s1s + rfs | ${node('x')}] AS nodes, + [r IN r1s + r2s + r3s + r4s + r5s + r6s + r7s + r8s + r9s | ${rel('r')}] AS relationships`; +} + +/** + * The content service's SummaryEntity: a reference entity written as one + * physical entity standing for all that refer to it, which /query/enhanced/v2 + * answers with when asked to summarise. Its constructor, in its order: + * + * - the reference entity's identity, names, database, links and cross + * references; a sequence's genes, checksum, comments, keywords, length and + * species; a gene product's chains, genes and transcripts; an isoform's + * variant and parent; + * - the physical entities, sorted -- in the reference entity's own list too, + * which is sorted in place -- by their modified residues (none for all but + * an EWAS), then dbId, as `summarisedEntities`; + * - then, entity by entity, each property merged in: a list joined to what + * is there already, each element once (LinkedHashSet), through the + * SummaryEntity's own getter and PhysicalEntity's setter -- which for the + * stoichiometric ones (compartment, componentOf, memberOf, candidateOf) + * expands and re-aggregates; a single value (species, referenceType) + * replaced by the entity's if it has one. + */ +export function summaryEntity(reference) { + const summary = new Entity(reference.id, 'SummaryEntity', {}); + const from = reference.fields; + const to = summary.fields; + const copy = (...names) => { + for (const name of names) to[name] = from[name]; + }; + copy('dbId', 'stId', 'displayName', 'name'); + to.referenceEntity = reference; + copy( + 'moleculeType', + 'databaseName', + 'identifier', + 'url', + 'referenceDatabase', + 'otherIdentifier', + 'crossReference' + ); + const is = (name) => isA(reference.schemaClass, name); + if (is('ReferenceSequence')) + copy( + 'geneName', + 'secondaryIdentifier', + 'checksum', + 'comment', + 'description', + 'keyword', + 'sequenceLength', + 'species', + 'isSequenceChanged' + ); + if (is('ReferenceGeneProduct')) copy('chain', 'referenceGene', 'referenceTranscript'); + if (is('ReferenceIsoform')) copy('variantIdentifier', 'isoformParent'); + + const residues = (entity) => + isA(entity.schemaClass, 'EntityWithAccessionedSequence') + ? (entity.fields.hasModifiedResidue ?? []).length + : 0; + const entities = from.physicalEntity ?? []; + entities.sort( + (a, b) => residues(a) - residues(b) || Number(a.fields.dbId) - Number(b.fields.dbId) + ); + to.summarisedEntities = entities; + + const union = (existing, added) => [...new Set([...(existing ?? []), ...added])]; + const unionRelations = (existing, added) => { + const out = [...(existing ?? [])]; + for (const relation of added) + if (!out.some((kept) => kept.element === relation.element)) out.push(relation); + return out; + }; + // PhysicalEntity's setter for a stoichiometric list: one relation per element, + // in the order given (Has.Util.aggregateStoichiometry). + const aggregate = (elements, relationClass) => + elements.map( + (element, order) => new Relation(relationClass, element, { order, stoichiometry: 1 }) + ); + const plain = [ + 'disease', + 'crossReference', + 'isRequired', + 'inferredFrom', + 'inferredTo', + 'catalystActivities', + 'literatureReference', + 'positivelyRegulates', + 'negativelyRegulates', + 'markingReferences', + ]; + const stoichiometric = { + compartment: 'HasCompartment', + componentOf: 'ComponentOf', + memberOf: 'MemberOf', + candidateOf: 'CandidateOf', + }; + // In the accessors' order (SummaryEntity.accessors). + const order = [ + 'disease', + 'crossReference', + 'compartment', + 'componentOf', + 'memberOf', + 'candidateOf', + 'isRequired', + 'inferredFrom', + 'inferredTo', + 'consumedByEvent', + 'producedByEvent', + 'catalystActivities', + 'literatureReference', + 'positivelyRegulates', + 'negativelyRegulates', + 'markingReferences', + ]; + for (const entity of entities) { + for (const name of order) { + if (plain.includes(name)) { + const value = entity.fields[name]; + if (!value?.length) continue; + to[name] = union(to[name], value); + } else if (name in stoichiometric) { + const value = expand(entity.fields[name]); + if (!value?.length) continue; + to[name] = aggregate(union(expand(to[name]), value), stoichiometric[name]); + } else { + // inputFor / outputFor: the relations themselves, one per element. + const value = entity.fields[name]; + if (!value?.length) continue; + to[name] = unionRelations(to[name], value); + } + } + if (isA(entity.schemaClass, 'GenomeEncodedEntity') || isA(entity.schemaClass, 'SimpleEntity')) { + if (entity.fields.species !== undefined && entity.fields.species !== null) + to.species = entity.fields.species; + } + if ( + isA(entity.schemaClass, 'SimpleEntity') || + isA(entity.schemaClass, 'EntityWithAccessionedSequence') + ) { + if (entity.fields.referenceType !== undefined && entity.fields.referenceType !== null) + to.referenceType = entity.fields.referenceType; + } + } + return summary; +} + /** * Nodes on their own, as SDN maps a query that returns only nodes: each with * its properties and no relationships, one instance per node, so a node that diff --git a/tools/content-node/domain.spec.mjs b/tools/content-node/domain.spec.mjs index c11f4bc2..89b98d10 100644 --- a/tools/content-node/domain.spec.mjs +++ b/tools/content-node/domain.spec.mjs @@ -10,6 +10,7 @@ import { javaDouble, mapNodes, mapResult, + summaryEntity, VIEW, writeDomain, } from './domain.mjs'; @@ -264,6 +265,89 @@ describe('writeDomain on a deep graph', () => { }); }); +describe("summaryEntity, the content service's SummaryEntity", () => { + // A protein referred to by two EWASs, the lower dbId with a modified residue: as + // /query/enhanced/v2?summariseReferenceEntity=true writes uniprot:P01112. + const EWAS = [ + 'DatabaseObject', + 'PhysicalEntity', + 'GenomeEncodedEntity', + 'EntityWithAccessionedSequence', + ]; + const row = { + root: node( + 70, + ['DatabaseObject', 'ReferenceEntity', 'ReferenceSequence', 'ReferenceGeneProduct'], + { + stId: 'uniprot:P01112', + displayName: 'UniProt:P01112 HRAS', + identifier: 'P01112', + name: ['HRAS'], + geneName: ['HRAS'], + } + ), + nodes: [ + node(72, EWAS, { displayName: 'p-HRAS', referenceType: 'ReferenceGeneProduct' }), + node(71, EWAS, { displayName: 'HRAS', referenceType: 'ReferenceGeneProduct' }), + node(80, ['DatabaseObject', 'Compartment', 'GO_CellularComponent'], { + displayName: 'plasma membrane', + }), + node(81, ['DatabaseObject', 'Compartment', 'GO_CellularComponent'], { + displayName: 'cytosol', + }), + node( + 90, + [ + 'DatabaseObject', + 'AbstractModifiedResidue', + 'TranslationalModification', + 'ModifiedResidue', + ], + {} + ), + node(95, ['DatabaseObject', 'Taxon', 'Species'], { displayName: 'Homo sapiens' }), + ], + relationships: [ + rel('referenceEntity', 72, 70), + rel('referenceEntity', 71, 70), + rel('compartment', 72, 80, { order: 0 }), + rel('compartment', 71, 81, { order: 0 }), + rel('compartment', 71, 80, { order: 1 }), + rel('hasModifiedResidue', 71, 90, { order: 0 }), + rel('species', 71, 95), + ], + }; + const summary = () => JSON.parse(writeDomain(summaryEntity(mapResult(row)))); + + it('is the reference entity under its own identity, written as a SummaryEntity', () => { + const json = summary(); + expect(json).toMatchObject({ + dbId: 70, + stId: 'uniprot:P01112', + schemaClass: 'SummaryEntity', + className: 'Protein', + }); + expect(json.identifier).toBe('P01112'); + expect(json.geneName).toEqual(['HRAS']); + }); + + it('lists the entities by modified residues, then dbId', () => { + // 71 has a modified residue, so it comes after 72 despite its lower dbId. + expect(summary().summarisedEntities.map((entity) => entity.dbId ?? entity)).toEqual([72, 71]); + }); + + it('merges their compartments once each, in the order met', () => { + // 72 brings plasma membrane; 71 cytosol, and plasma membrane again. + expect(summary().compartment.map((entity) => entity.dbId ?? entity)).toEqual([80, 81]); + }); + + it("takes an entity's species and reference type when it has one", () => { + const json = summary(); + expect(json.species).toEqual(95); + expect(json.referenceType).toBe('ReferenceGeneProduct'); + }); +}); + describe('mapNodes, a query that returns nodes alone', () => { const book = node(30, ['DatabaseObject', 'Publication', 'Book'], { displayName: 'b' }); it('writes each node with its properties and no relationships', () => { diff --git a/tools/content-node/service.mjs b/tools/content-node/service.mjs index c0eacbda..218d4b64 100644 --- a/tools/content-node/service.mjs +++ b/tools/content-node/service.mjs @@ -24,13 +24,16 @@ import { fileURLToPath, URLSearchParams } from 'node:url'; import neo4j from 'neo4j-driver'; import { read, configured, close } from './graph.mjs'; import { + enhancedQuery, findByIdParameters, findByIdQuery, inDepthQuery, + isA, mapNodes, oldEnhancedQuery, mapResult, rootLabelsQuery, + summaryEntity, VIEW, writeDomain, } from './domain.mjs'; @@ -665,6 +668,62 @@ export const endpoints = [ return { status: 200, body, type: 'application/json' }; }), })), + { + path: '/ContentService/data/query/enhanced/v2/{id}', + sample: + 'R-HSA-70486?fetchIncomingRelationships=true&summariseReferenceEntity=true&includeDisease=true&includeRef=true&view=nested-aggregated', + differs: [/^\/ContentService\/data\/query\/enhanced\/v2\/[^/]+[^:]*: \.url: /], + /** + * An object with everything around it, for the Pathway Browser's details + * panel and the website's detail pages -- the site's busiest content + * call. graph-core's findEnhancedObjectById (enhancedQuery), written by + * the domain serialiser in the request's view; a reference entity, when + * asked to summarise, as the content service's SummaryEntity + * (summaryEntity). + * + * As QueryObjectController: three primitive booleans with defaults + * (fetchIncomingRelationships true, summariseReferenceEntity false, + * includeDisease true); incoming relationships only when asked for and + * needsIncoming agrees; an object found only with its disease data, + * asked for without, looked up again with it. + */ + handler: async (request) => { + const raw = pathId(rawSegment(request, 6)); + const flags = {}; + for (const [name, fallback] of [ + ['fetchIncomingRelationships', true], + ['summariseReferenceEntity', false], + ['includeDisease', true], + ]) { + const parsed = springPrimitiveBoolean(queryValues(request, name), fallback); + if (parsed.error) return javaError(request, 400, 'BAD_REQUEST', parsed.error); + flags[name] = parsed.value; + } + const id = inDepthId(raw); + const missing = () => notFound(request, `Id: ${raw} has not been found in the System`); + if (!id) return missing(); + const options = { + idType: id.idType, + summarise: flags.summariseReferenceEntity, + includeDisease: flags.includeDisease, + outgoingOnly: !(flags.fetchIncomingRelationships && (await needsIncoming(raw))), + }; + let [row] = await read(enhancedQuery(options), { id: id.id }); + if (!row && !options.includeDisease) + [row] = await read(enhancedQuery({ ...options, includeDisease: true }), { id: id.id }); + if (!row) return missing(); + const found = mapResult(row); + const object = + flags.summariseReferenceEntity && isA(found.schemaClass, 'ReferenceEntity') + ? summaryEntity(found) + : found; + return { + status: 200, + serialised: writeDomain(object, ...writerFor(request)), + type: 'application/json', + }; + }, + }, { // Before /query/{id}/stId and /normalPathway, as Spring ranks its literal // `enhanced` above their {id}: /query/enhanced/stId is this, with id stId. @@ -2772,6 +2831,26 @@ export function inDepthAttributes(values, fallback = 'species,compartment') { return { value: joined }; } +/** + * A primitive `boolean` request parameter with a default, as Spring reads it: + * absent or empty takes the default; otherwise as springBoolean reads a + * Boolean, except that blank is no longer "absent" but a value Spring cannot + * put in a primitive -- its 400, naming `boolean`. Measured against Java's + * /query/enhanced/v2. + */ +export function springPrimitiveBoolean(values, fallback) { + if (values.length === 0 || values[0] === '') return { value: fallback }; + const value = javaTrim(values[0]).toLowerCase(); + if (['true', 'on', 'yes', '1'].includes(value)) return { value: true }; + if (['false', 'off', 'no', '0'].includes(value)) return { value: false }; + const type = values.length > 1 ? 'java.lang.String[]' : 'java.lang.String'; + return { + error: + `Failed to convert value of type '${type}' to required type 'boolean'; ` + + `nested exception is java.lang.IllegalArgumentException: Invalid boolean value [${values.join(',')}]`, + }; +} + /** * A primitive `int` request parameter with a default, as Spring reads it: * absent or empty takes the default; otherwise the first value parsed as diff --git a/tools/content-node/service.spec.mjs b/tools/content-node/service.spec.mjs index 99bd0b19..4f755096 100644 --- a/tools/content-node/service.spec.mjs +++ b/tools/content-node/service.spec.mjs @@ -30,6 +30,7 @@ import { inDepthAttributes, inDepthId, springIntWithDefault, + springPrimitiveBoolean, personWhere, PathwayBrowserNode, pathwayBrowserJson, @@ -392,6 +393,24 @@ describe("needsIncoming, QueryObjectController's needsIncomingRelationship", () }); }); +describe('springPrimitiveBoolean, a boolean with a default as Spring reads it', () => { + // Measured against Java's /query/enhanced/v2, 15 Oct 2026. + it('takes the default for absent or empty, and reads the words Spring knows', () => { + expect(springPrimitiveBoolean([], true)).toEqual({ value: true }); + expect(springPrimitiveBoolean([''], false)).toEqual({ value: false }); + expect(springPrimitiveBoolean(['TRUE'], false)).toEqual({ value: true }); + expect(springPrimitiveBoolean(['off'], true)).toEqual({ value: false }); + }); + + it("is Spring's 400 for blank or anything else, naming boolean", () => { + expect(springPrimitiveBoolean([' '], true).error).toBe( + "Failed to convert value of type 'java.lang.String' to required type 'boolean'; " + + 'nested exception is java.lang.IllegalArgumentException: Invalid boolean value [ ]' + ); + expect(springPrimitiveBoolean(['x', 'y'], true).error).toMatch(/String\[\]' .*\[x,y\]$/); + }); +}); + describe('the in-depth parameters, as the in-depth controllers read them', () => { // Each measured against Java's /data/entity/{id}/in-depth, 15 Oct 2026. it('reads maxDepth as a primitive int with a default', () => { diff --git a/tools/content-node/snapshot-domain.mjs b/tools/content-node/snapshot-domain.mjs index f9d66b09..2f009880 100644 --- a/tools/content-node/snapshot-domain.mjs +++ b/tools/content-node/snapshot-domain.mjs @@ -83,6 +83,10 @@ try { ); // Has.Util.wrapUniqueElements wraps a plain relationship as this, for composedOf. const wrapper = `${DOMAIN}relationship/Has$Default`.replaceAll('/', '.'); + // The content service's own: what /query/enhanced/v2 writes a reference + // entity as (domain.mjs, summaryEntity). In the webapp's classes, not a jar. + const CLASSES = path.join(LIB, '..', 'classes'); + const summary = 'org.reactome.server.service.model.graph.SummaryEntity'; const build = path.join(work, 'build'); run('javac', [ @@ -94,10 +98,28 @@ try { path.join(HERE, 'snapshot-domain', 'DomainSnapshot.java'), ]); const dump = JSON.parse( - run('java', ['-cp', `${build}${path.delimiter}${LIB}/*`, 'DomainSnapshot', ...classes, wrapper]) + run('java', [ + '-cp', + [build, CLASSES, `${LIB}/*`].join(path.delimiter), + 'DomainSnapshot', + ...classes, + wrapper, + '--serialised-only', + summary, + ]) ); - const bodies = methodBodies(run('javap', ['-c', '-p', '-cp', work, ...classes, wrapper])); + const bodies = methodBodies( + run('javap', [ + '-c', + '-p', + '-cp', + [work, CLASSES].join(path.delimiter), + ...classes, + wrapper, + summary, + ]) + ); const computed = new Set(COMPUTED_GETTERS); const json = {}; for (const [schemaClass, properties] of Object.entries(dump.json)) { @@ -112,8 +134,8 @@ try { throw new Error( `${schemaClass}.${name}: ${key} computes its value; implement it in domain.mjs` ); - // Has.Default is built in code, not mapped from the graph. - if (source === field && schemaClass !== 'Default') { + // Has.Default and SummaryEntity are built in code, not mapped from the graph. + if (source === field && schemaClass !== 'Default' && schemaClass !== 'SummaryEntity') { const mapped = dump.graph[schemaClass]?.fields.some(([fieldName]) => fieldName === field); if (!mapped) throw new Error(`${schemaClass}.${name}: reads ${field}, which SDN does not map`); diff --git a/tools/content-node/snapshot-domain/DomainSnapshot.java b/tools/content-node/snapshot-domain/DomainSnapshot.java index a6266258..be4ed4ae 100644 --- a/tools/content-node/snapshot-domain/DomainSnapshot.java +++ b/tools/content-node/snapshot-domain/DomainSnapshot.java @@ -50,7 +50,20 @@ public abstract static class DatabaseObjectMixin { public static void main(String[] args) throws Exception { List> classes = new ArrayList<>(); - for (String name : args) classes.add(Class.forName(name)); + // Classes after "--serialised-only" are written but never mapped: the + // content service's own (SummaryEntity), built in code from mapped ones. + // Kept out of SDN's mapping context, which they would join and reorder. + Set> serialisedOnly = new LinkedHashSet<>(); + boolean only = false; + for (String name : args) { + if (name.equals("--serialised-only")) { + only = true; + continue; + } + Class type = Class.forName(name); + classes.add(type); + if (only) serialisedOnly.add(type); + } ObjectMapper mapper = new ObjectMapper(); mapper.setSerializationInclusion(JsonInclude.Include.NON_EMPTY); @@ -63,7 +76,8 @@ public static void main(String[] args) throws Exception { Neo4jMappingContext context = new Neo4jMappingContext(); // Has.Default is only serialized: it is built in code, never mapped. Set> mapped = new LinkedHashSet<>(); - for (Class type : classes) if (!type.isMemberClass()) mapped.add(type); + for (Class type : classes) + if (!type.isMemberClass() && !serialisedOnly.contains(type)) mapped.add(type); context.setInitialEntitySet(mapped); context.initialize(); From 855018d76f2de09f098bd86f888730196ac2007b Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Thu, 15 Oct 2026 15:29:03 +0000 Subject: [PATCH 2/2] Review: only a single empty parameter is Spring's default Review of #476. Spring uses a parameter's defaultValue only when the argument is exactly "". A repeated parameter arrives as a String[]: an empty first element cannot become a primitive, so Spring converts the joined values and fails. Java answers ?includeDisease=&includeDisease=true with a 400 "Invalid boolean value [,true]", and ?maxDepth=&maxDepth=2 (in-depth) with a 400 "For input string: ",2"". Node took the defaults. springPrimitiveBoolean and springIntWithDefault now default only for no value or a single empty one. 4 of 4 cases match Java. Tests failed before the change. Co-Authored-By: Claude Opus 5.5 --- tools/content-node/service.mjs | 6 ++++-- tools/content-node/service.spec.mjs | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/content-node/service.mjs b/tools/content-node/service.mjs index 218d4b64..57db52d1 100644 --- a/tools/content-node/service.mjs +++ b/tools/content-node/service.mjs @@ -2839,7 +2839,8 @@ export function inDepthAttributes(values, fallback = 'species,compartment') { * /query/enhanced/v2. */ export function springPrimitiveBoolean(values, fallback) { - if (values.length === 0 || values[0] === '') return { value: fallback }; + // Only one empty value is Spring's "absent": repeated, the array is converted. + if (values.length === 0 || (values.length === 1 && values[0] === '')) return { value: fallback }; const value = javaTrim(values[0]).toLowerCase(); if (['true', 'on', 'yes', '1'].includes(value)) return { value: true }; if (['false', 'off', 'no', '0'].includes(value)) return { value: false }; @@ -2858,7 +2859,8 @@ export function springPrimitiveBoolean(values, fallback) { * failing both, the 400 names `int` and the second attempt's message. */ export function springIntWithDefault(values, fallback) { - if (values.length === 0 || values[0] === '') return { value: fallback }; + // Only one empty value is Spring's "absent": repeated, the array is converted. + if (values.length === 0 || (values.length === 1 && values[0] === '')) return { value: fallback }; const attempt = (text) => { try { return { value: javaParseInteger(text) }; diff --git a/tools/content-node/service.spec.mjs b/tools/content-node/service.spec.mjs index 4f755096..5e1a6030 100644 --- a/tools/content-node/service.spec.mjs +++ b/tools/content-node/service.spec.mjs @@ -408,6 +408,10 @@ describe('springPrimitiveBoolean, a boolean with a default as Spring reads it', 'nested exception is java.lang.IllegalArgumentException: Invalid boolean value [ ]' ); expect(springPrimitiveBoolean(['x', 'y'], true).error).toMatch(/String\[\]' .*\[x,y\]$/); + // Only a single empty value is the default; repeated, the first being + // empty, Spring converts the array and fails (review of #476). + expect(springPrimitiveBoolean(['', 'true'], false).error).toMatch(/String\[\]' .*\[,true\]$/); + expect(springPrimitiveBoolean(['', ''], false).error).toMatch(/\[,\]$/); }); }); @@ -418,6 +422,10 @@ describe('the in-depth parameters, as the in-depth controllers read them', () => expect(springIntWithDefault([''], 1)).toEqual({ value: 1 }); expect(springIntWithDefault(['0x2'], 1)).toEqual({ value: 2 }); expect(springIntWithDefault(['1', '2'], 1)).toEqual({ value: 1 }); + expect(springIntWithDefault(['', '2'], 1).error).toBe( + "Failed to convert value of type 'java.lang.String[]' to required type 'int'; " + + 'nested exception is java.lang.NumberFormatException: For input string: ",2"' + ); expect(springIntWithDefault(['x'], 1).error).toBe( "Failed to convert value of type 'java.lang.String' to required type 'int'; " + 'nested exception is java.lang.NumberFormatException: For input string: "x"'