diff --git a/deploy/java/PINS.md b/deploy/java/PINS.md index e3383fbaf..36f87eab9 100644 --- a/deploy/java/PINS.md +++ b/deploy/java/PINS.md @@ -38,7 +38,7 @@ The Reactome Java this deployment builds (spec 010), each pinned to the commit t | Artifact | Repository | Commit | Runtime | | -------------------- | --------------------- | ------------------ | ------- | -| pdf-export (`serve`) | `reactome/pdf-export` | `979b8d6` (`main`) | Node 24 | +| pdf-export (`serve`) | `reactome/pdf-export` | `55cde47` (`main`) | Node 24 | ## Changing a pin diff --git a/deploy/nginx/common/routes-core.conf b/deploy/nginx/common/routes-core.conf index 813a29ff8..f5337e217 100644 --- a/deploy/nginx/common/routes-core.conf +++ b/deploy/nginx/common/routes-core.conf @@ -648,6 +648,12 @@ location /AnalysisService/ { # No URI part, as for /ContentService/: the path as sent, never nginx's # decoded copy of it. proxy_pass http://analysis; + # Who the client is, for pdf-export's share of the waiting reports + # (pdf-export spec 002, T021): always the address nginx saw, whatever a + # request sent, as pdf-export trusts it. The analysis gateway sets it + # again from its own reading (an IPv6 client by its /64); this keeps a + # value a client chose from passing through where it does not. + proxy_set_header X-Reactome-Client $remote_addr; # An analysis over a large identifier list is not quick either. proxy_read_timeout 300s; proxy_send_timeout 300s; diff --git a/deploy/nginx/test-config.sh b/deploy/nginx/test-config.sh index f642f690c..aa24f9059 100755 --- a/deploy/nginx/test-config.sh +++ b/deploy/nginx/test-config.sh @@ -14,11 +14,51 @@ cd "$(dirname "$0")/../.." work=$(mktemp -d) images=() +containers=() # The images too: on a shared host they are a few hundred megabytes each. -trap 'rm -rf "$work"; [ ${#images[@]} -eq 0 ] || docker rmi -f "${images[@]}" >/dev/null 2>&1' EXIT +trap '[ ${#containers[@]} -eq 0 ] || docker rm -f "${containers[@]}" >/dev/null 2>&1; rm -rf "$work"; [ ${#images[@]} -eq 0 ] || docker rmi -f "${images[@]}" >/dev/null 2>&1' EXIT openssl req -x509 -nodes -newkey rsa:2048 -days 1 -subj /CN=localhost \ -keyout "$work/key.pem" -out "$work/cert.pem" 2>/dev/null +# The client the analysis report is counted by (pdf-export spec 002, T021): +# pdf-export trusts X-Reactome-Client for its share of the waiting reports, so +# nginx must set it from the address it saw and never pass on one a request +# chose. Run for real: the deployment's image, its HTTPS server, and a stand-in +# for its `analysis` upstream that answers with the headers it was sent. Both +# share a network namespace with no network, so nothing on this host or +# beyond is reached. A request naming another client in every header a +# client could use is still counted by its own address (here, loopback). +client_named() { + local env=$1 image=$2 + shift 2 + local conf="deploy/nginx/$env.conf" host port stub server answer + # The server that serves the site's routes, and the analysis upstream's port. + host=$(awk '/server_name/ { n = $2 } /common\/routes\.conf/ { sub(/;$/, "", n); print n; exit }' "$conf") + port=$(awk '/^upstream analysis \{/ { a = 1 } a && /^[[:space:]]*server[[:space:]]/ { sub(/;.*/, ""); n = split($2, p, ":"); print p[n]; exit }' "$conf") + if [ -z "$host" ] || [ -z "$port" ]; then + echo "no site server or analysis upstream found in $conf" + return 1 + fi + printf 'server {\n listen 127.0.0.1:%s;\n location / {\n default_type text/plain;\n return 200 "client=$http_x_reactome_client real=$http_x_real_ip";\n }\n}\n' \ + "$port" >"$work/stub-$env.conf" + stub=$(docker run -d --network none -v "$work/stub-$env.conf:/etc/nginx/conf.d/default.conf:ro" nginx:alpine) + containers+=("$stub") + server=$(docker run -d --network "container:$stub" "$@" "$image") + containers+=("$server") + for _ in $(seq 50); do + answer=$(docker exec "$stub" curl -sk -m 5 --resolve "$host:443:127.0.0.1" \ + -H 'X-Reactome-Client: 203.0.113.9' -H 'X-Real-IP: 203.0.113.9' \ + -H 'X-Forwarded-For: 203.0.113.9' -H 'CF-Connecting-IP: 203.0.113.9' \ + "https://$host/AnalysisService/report/T/9606/report.pdf" 2>/dev/null) && break + sleep 0.2 + done + if [ "$answer" != 'client=127.0.0.1 real=127.0.0.1' ]; then + echo "the analysis service was sent: ${answer:-nothing}" + docker logs "$server" 2>&1 | tail -5 + return 1 + fi +} + status=0 for env in dev release production; do # Every certificate and key the configuration names, made under $work. @@ -46,6 +86,14 @@ for env in dev release production; do echo "FAIL $env" sed 's/^/ /' "$work/out" status=1 + continue + fi + if client_named "$env" "$image" "${mounts[@]}" >"$work/out" 2>&1; then + echo "ok $env: the analysis service is told the client's own address" + else + echo "FAIL $env: a client chose the address it is counted by" + sed 's/^/ /' "$work/out" + status=1 fi done exit $status diff --git a/deploy/pdf-export/Dockerfile b/deploy/pdf-export/Dockerfile index 4b2f67fc6..5631e125c 100644 --- a/deploy/pdf-export/Dockerfile +++ b/deploy/pdf-export/Dockerfile @@ -3,6 +3,9 @@ # for an event's PDF at /ContentService/exporter/document/event/{id}.pdf. It # reads the graph, draws its figures through the render service, reads an # analysis token's results from the AnalysisService, and prints with Chromium. +# It also answers the analysis report, /AnalysisService/report/... (pdf-export +# spec 002), which the analysis gateway sends it; its printer's working folder +# is compose's tmpfs (PDF_EXPORT_WORK). # # Built from the commit deploy/java/PINS.md records ("The PDF service"), from # GitHub's archive of it: nothing but that commit. pins.spec.ts checks the two @@ -12,7 +15,7 @@ # browser, and node:24's layers are already on the host. FROM node:24 -ARG PDF_EXPORT_COMMIT=979b8d6d436c1edcf7e54e104c04c617f12692a8 +ARG PDF_EXPORT_COMMIT=55cde47f05832123a685b8c293bb3665465cb32f # Where the browser goes: outside any user's home, as the service runs as the # owner of the graph's credentials file, not as the user that installed it diff --git a/deploy/pdf-export/compose.spec.ts b/deploy/pdf-export/compose.spec.ts index bda7a2be9..b894a3d32 100644 --- a/deploy/pdf-export/compose.spec.ts +++ b/deploy/pdf-export/compose.spec.ts @@ -52,6 +52,35 @@ describe('the PDF service in compose', () => { expect(dockerfile).toMatch(/"--files", "\/files"/); }); + it('keeps the printer’s working folder in memory, its user’s alone (pdf-export spec 002, T018)', () => { + // A report's page holds what a user submitted, and one killed mid-print + // stays until the service starts again: never on disk. + expect(pdf).toMatch(/^ {6}- PDF_EXPORT_WORK=\/work$/m); + const mount = pdf.match(/^ {4}tmpfs:\n {6}- \/work:(\S+)$/m)?.[1] ?? 'no tmpfs at /work'; + const options = mount.split(','); + expect(options).toEqual( + expect.arrayContaining([ + 'mode=0700', + 'uid=${CONTENT_NODE_UID:-1020}', + 'gid=${CONTENT_NODE_GID:-1001}', + 'noexec', + ]) + ); + expect(options.some((o) => /^size=\d+m$/.test(o))).toBe(true); + }); + + it('reads the analysis service directly, not through the gateway that sends it reports', () => { + expect(pdf).toMatch(/^ {6}- ANALYSIS_SERVICE=http:\/\/127\.0\.0\.1:8080\/AnalysisService$/m); + }); + + it('is where the analysis gateway sends the reports (pdf-export spec 002, T021)', () => { + const gateway = service('analysis-gateway'); + expect(gateway).toMatch( + /^ {6}- REPORT_UPSTREAM=\$\{REPORT_UPSTREAM:-http:\/\/127\.0\.0\.1:4370\}$/m + ); + expect(gateway).not.toMatch(/REPORT_LIMIT/); + }); + it('gives Chromium room, and the whole a limit', () => { expect(pdf).toMatch(/^ {4}shm_size: '1gb'$/m); expect(pdf).toMatch(/^ {4}mem_limit: \d+[mg]$/m); diff --git a/docker-compose.yml b/docker-compose.yml index 60e4e673d..86d0e5d3e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -247,6 +247,9 @@ services: # through unchanged. Host networking, as content-node: Tomcat's # AnalysisService is on the host's loopback, and nginx reaches this on # 127.0.0.1:4350. Stateless, so a restart costs only the requests in flight. + # + # The analysis report PDF goes to pdf-export instead (pdf-export spec 002, + # T021), unqueued, with the client named in X-Reactome-Client. analysis-gateway: build: context: . @@ -255,8 +258,8 @@ services: network_mode: host environment: - ANALYSIS_UPSTREAM=${ANALYSIS_UPSTREAM:-http://127.0.0.1:8080} + - REPORT_UPSTREAM=${REPORT_UPSTREAM:-http://127.0.0.1:4370} - ANALYSIS_LIMIT=${ANALYSIS_LIMIT:-4} - - REPORT_LIMIT=${REPORT_LIMIT:-1} - QUEUE_LENGTH=${QUEUE_LENGTH:-40} - PER_CLIENT=${PER_CLIENT:-8} - MAX_WAIT_MS=${MAX_WAIT_MS:-60000} @@ -380,8 +383,10 @@ services: # pdf-export's single-event service (pdf-export spec 001, Story 2) from the # graph, its figures drawn by the render service as the website draws them, # an analysis token's results read from the AnalysisService. It replaces - # Java's event-pdf for the website. Node is its only client; nginx never - # names it. + # Java's event-pdf for the website, and, through the analysis gateway, the + # AnalysisService's report PDF (pdf-export spec 002). Node and the gateway + # are its only clients; nginx never names it, as it trusts the client the + # gateway names in X-Reactome-Client. # # Built here from the commit deploy/java/PINS.md pins, from GitHub's archive # of it, and never pulled from or pushed to a registry: no `image:`. @@ -415,15 +420,28 @@ services: # Every figure drawn on request by the render service: no figures' # folder (pdf-export's `figures`, contracts/figures.md) is given yet. - RENDER_SERVICE=http://127.0.0.1:4310 - # Read for a PDF with an analysis token: the token's results, never a - # new analysis, so not through the analysis gateway's queue. + # Read for a PDF with an analysis token, and for the analysis report + # (pdf-export spec 002, contracts/analysis-api.md): the token's results, + # never a new analysis, so read from the AnalysisService directly, not + # through the analysis gateway, which sends the report here. - ANALYSIS_SERVICE=http://127.0.0.1:8080/AnalysisService + # The printer's working folder (pdf-export spec 002, FR-211, research + # R7 and R10): a report's page holds the identifiers a user submitted, + # and one killed while it printed leaves its page behind until the + # service starts again. So it is the tmpfs below, in memory and never + # on disk, owned by the service's user alone; the service empties it + # as it starts. + - PDF_EXPORT_WORK=/work # Where the PDF's links go: the public site, as event-pdf's do. - REACTOME_SITE=${PDF_SITE:-https://reactome.org} + tmpfs: + - /work:size=256m,mode=0700,uid=${CONTENT_NODE_UID:-1020},gid=${CONTENT_NODE_GID:-1001},noexec,nosuid,nodev # Chromium prints the PDFs; its default 64 MB /dev/shm is too small for a # large page, as for the render service. shm_size: '1gb' - mem_limit: 2g + # Within it: the reports kept for an hour (512 MB at most, pdf-export's + # ServeLimits), two printing at once, and the working folder's tmpfs. + mem_limit: 3g healthcheck: test: ['CMD', 'curl', '-fs', '-m', '10', '-o', '/dev/null', 'http://127.0.0.1:4370/health'] interval: 30s diff --git a/documentation/declared-differences.md b/documentation/declared-differences.md index 38a161041..c9559171b 100644 --- a/documentation/declared-differences.md +++ b/documentation/declared-differences.md @@ -12,7 +12,7 @@ reports the difference as intended and reports its disappearance as news This page gathers every declared difference into one register. It does not replace the sources: each row links to the place the difference is recorded, -which stays the authority. Compiled 6 Nov 2026, and updated since (SBGN and the website's event PDF, 7 Nov 2026; search, diagram and reaction images, PowerPoint, 8 Nov 2026; the analysis report's comparisons, 14 Nov 2026). +which stays the authority. Compiled 6 Nov 2026, and updated since (SBGN and the website's event PDF, 7 Nov 2026; search, diagram and reaction images, PowerPoint, 8 Nov 2026; the analysis report's comparisons and its routing to pdf-export, 14 Nov 2026). **Terms used here** @@ -45,13 +45,13 @@ which stays the authority. Compiled 6 Nov 2026, and updated since (SBGN and the | [ContentService: reaction layout](#contentservice-reaction-layout) | 4 | Live on beta | | [ContentService: contact form](#contentservice-contact-form) | 7 | Live on beta | | [ContentService: images](#contentservice-images) | 14 | Live on beta, but the PowerPoint and the overview's options, merged | -| [AnalysisService gateway](#analysisservice-gateway) | 2 | Live on beta | +| [AnalysisService gateway](#analysisservice-gateway) | 5 | Two live on beta; the report's three merged | | [SBML (sbml-exporter)](#sbml-sbml-exporter) | 13 | Website's SBML live on beta; bulk file not switched | | [BioPAX (BioPAX3Exporter)](#biopax-biopax3exporter) | 13 | Exporter merged; website endpoint live on beta | | [SBGN (sbgn-exporter)](#sbgn-sbgn-exporter) | 8 | Website's SBGN merged, routed on beta; bulk file not switched | | [Pathway PDFs (pdf-export spec 001)](#pathway-pdfs-pdf-export-spec-001) | 17 | Website's PDF live on beta; book not switched | -| [Analysis report PDF (pdf-export spec 002)](#analysis-report-pdf-pdf-export-spec-002) | 19 | Merged; compared on the development host; website not routed to it | -| **Total** | **111** | | +| [Analysis report PDF (pdf-export spec 002)](#analysis-report-pdf-pdf-export-spec-002) | 21 | Merged; website routed to it (spec 010 T066), not yet deployed | +| **Total** | **116** | | Then: [things that look like differences but are not](#not-differences), and [decisions still open](#decisions-still-open). @@ -143,12 +143,17 @@ sending, and the message's subject and body are kept. Done on beta on The analysis maths stays in Java. A small gateway in this repository sits in front of the AnalysisService and admits a few computations at a time. Every -answer the service gives passes through byte for byte. - -| What | Java | New | Why | Decided | Status | Source | -| --------------------- | ----------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------- | ---------------------------- | ------------ | --------------------------------------------------------------------------- | -| Many analyses at once | Runs them all at once; under a burst every one slows, and all can pass the edge's 100-second limit together | At most 4 analyses (and 1 PDF report) run at a time, the rest wait in order. A request that cannot be admitted in time is a `503` with `Retry-After`, in the service's error form, and is never run later. The site retries once | Design: a burst should make a few people wait, not fail everyone | The user, 19 Oct 2026 (R12) | Live on beta | [spec 010 R12, FR-021–FR-024](../specs/010-java-services/research.md), #539 | -| A share per client | None | At most 8 computations running or waiting per client address (an IPv6 one by its /64) | Design: one client cannot keep everyone else out | Review of 20 Oct 2026 (T065) | Live on beta | [spec 010 T065](../specs/010-java-services/tasks.md), #574 | +answer the service gives passes through byte for byte. The analysis report +PDF is not the AnalysisService's any more: the gateway sends it to pdf-export +([below](#analysis-report-pdf-pdf-export-spec-002); spec 010 T066). + +| What | Java | New | Why | Decided | Status | Source | +| -------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Many analyses at once | Runs them all at once; under a burst every one slows, and all can pass the edge's 100-second limit together | At most 4 analyses run at a time, the rest wait in order. A request that cannot be admitted in time is a `503` with `Retry-After`, in the service's error form, and is never run later. The site retries once. (Until T066, also 1 PDF report at a time.) | Design: a burst should make a few people wait, not fail everyone | The user, 19 Oct 2026 (R12) | Live on beta | [spec 010 R12, FR-021–FR-024](../specs/010-java-services/research.md), #539 | +| A share per client | None | At most 8 computations running or waiting per client address (an IPv6 one by its /64) | Design: one client cannot keep everyone else out | Review of 20 Oct 2026 (T065) | Live on beta | [spec 010 T065](../specs/010-java-services/tasks.md), #574 | +| The analysis report PDF, `GET /AnalysisService/report/…` | Made by the AnalysisService; the gateway queued it, one at a time | Sent to pdf-export, not queued; counted in the client's share of 8 and refused (`503`) past it; pdf-export told the client in `X-Reactome-Client`, set by nginx and again by the gateway, never the request's own | Design: pdf-export holds a request up to 30 s while it makes the report in the background, so a queue of one would make every browser asking again wait for the others; pdf-export has its own limits | pdf-export spec 002 R6 (reviewed 13 Nov 2026) | Merged | [spec 010 T066](../specs/010-java-services/tasks.md), [pdf-export spec 002 T021](https://github.com/reactome/pdf-export/blob/main/specs/002-analysis-report/tasks.md) | +| The report's service cannot be reached | The gateway's `502` (`Bad Gateway`) for the AnalysisService | `503` with `Retry-After`, in the analysis service's JSON form, as pdf-export writes its own `503` | Design: the report's address keeps one error format (pdf-export spec 002, FR-203) | pdf-export spec 002 R6 | Merged | [spec 010 T066](../specs/010-java-services/tasks.md) | +| A `HEAD` of the report's address | The framework runs the `GET` handler: the report is made, its headers answered (read from the framework, not measured) | pdf-export's `405` with `Allow: GET` and no body | Design: a `HEAD` would otherwise reach the AnalysisService's report outside any queue, and pdf-export answers `GET` alone | T066, 14 Nov 2026 | Merged | [spec 010 T066](../specs/010-java-services/tasks.md) | ## SBML (sbml-exporter) @@ -271,7 +276,12 @@ The analysis report PDF, today written inside the AnalysisService by `analysis-report`, is to be made by pdf-export with today's content in the new look (the user's choice, 30 Oct 2026). **Merged** in pdf-export and compared with today's report on the development host's own services (spec 002 T019, -T020; research R9, R10); the website is not routed to it yet (T021). Source: +T020; research R9, R10); **the website is routed to it** through the analysis +gateway (spec 002 T018 and T021, spec 010 T066), which takes effect on beta +when deployed. Its error answers were compared again through the gateway, on +the development host, for T001's error cases: the same status, `Content-Type` +and body as the analysis service's, but the two listed below (a malformed +token). Source: [pdf-export spec 002](https://github.com/reactome/pdf-export/blob/main/specs/002-analysis-report/spec.md), Assumptions ("Expected differences from today's report"). @@ -303,6 +313,8 @@ differ by T019, was a bug in the new report and now matches today's. | A malformed token, or one holding `%2F` | 500, or the servlet container's HTML 400 | 404 in the analysis service's JSON form | Bug fix | | `importableOnly` of spaces alone | 500 with an empty body (measured) | Read as the default (false) | Bug fix | | The PDF's `Content-Type` | `application/pdf;charset=UTF-8` | `application/pdf` | Bug fix (a PDF has no charset) | +| Each report's timing, with the client's address and browser | Sent to a separate reporting service | Not sent; one log line per report, with no submitted identifier or client | Design (FR-211) | +| The website's "PDF Result" button | Downloads the address's answer | Opens the address in a new tab, whose page asks again until the PDF downloads | Design (spec 002 plan, step 8; T021) | ## Not differences @@ -356,9 +368,9 @@ These show up in comparisons but are not differences in what a reader gets. - **The analysis report's figures**: the render page now waits for an analysis overlay (pdf-export T017, #771, #958); it must still take a resource before the report can match today's - overlays (pdf-export spec 002, FR-204). The report's species overview still - shows the retired Fireworks figure while the AnalysisService makes it - (spec 010 T058). + overlays (pdf-export spec 002, FR-204). The retired Fireworks figure in the + report's species overview (spec 010 T058) goes when the report is routed to + pdf-export, which draws the overview with the render service. ## Inconsistencies between sources diff --git a/e2e/analysis-results.spec.ts b/e2e/analysis-results.spec.ts index 7ebfdd401..d84805550 100644 --- a/e2e/analysis-results.spec.ts +++ b/e2e/analysis-results.spec.ts @@ -96,7 +96,22 @@ test.describe('Analysis results', () => { await expect.poll(pathways, { message: 'but not none of them' }).toBeGreaterThan(0); }); - test('the result files download', async ({ page }) => { + test('the result files download', async ({ page, context }) => { + // The PDF report is made in the background by pdf-export (its spec 002): + // its address answers "being made" until the report is ready. Fetched and + // saved by the page, that answer was saved as report.pdf; opened in a tab, + // its page asks again and the PDF downloads once ready. Stubbed here, so + // no report is made. + const reportsAsked: string[] = []; + await context.route('**/AnalysisService/report/**', (route) => { + reportsAsked.push(route.request().url()); + return route.fulfill({ + status: 202, + headers: { 'Retry-After': '10', 'Cache-Control': 'no-store' }, + contentType: 'text/html;charset=UTF-8', + body: 'Making your report

Making your report

', + }); + }); await runGeneList(page); await openTab(page, 'Download'); @@ -113,6 +128,22 @@ test.describe('Analysis results', () => { expect(bytes.length, `${label} size`).toBeGreaterThan(100); expect(looksRight(bytes), `${label} content`).toBe(true); } + + // The PDF report opens in a new tab, and is not downloaded by the page. + let downloads = 0; + page.on('download', () => downloads++); + const [tab] = await Promise.all([ + context.waitForEvent('page', { timeout: 30_000 }), + page.getByText('PDF Result', { exact: true }).click(), + ]); + await tab.waitForLoadState(); + expect(new URL(tab.url()).pathname).toMatch( + /^\/AnalysisService\/report\/[^/]+\/[^/]+\/report\.pdf$/ + ); + await expect(tab.getByRole('heading', { name: 'Making your report' })).toBeVisible(); + // Asked once, by the new tab: the page itself fetched nothing. + expect(reportsAsked).toHaveLength(1); + expect(downloads).toBe(0); }); test('species comparison overlays the pathways', async ({ page }) => { diff --git a/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.html b/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.html index 3000aa2c5..3f6a4c4ea 100644 --- a/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.html +++ b/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.html @@ -3,13 +3,13 @@ diff --git a/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.ts b/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.ts index 20556332d..1e2544e7c 100644 --- a/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.ts +++ b/projects/pathway-browser/src/app/details/tabs/download-tab/download-button/download-button.component.ts @@ -46,6 +46,12 @@ export class DownloadButtonComponent { readonly label = input(); readonly tooltip = input(); readonly openInNewTab = input('_blank'); + /** + * Opens the address in a new tab instead of downloading it: for a file made + * in the background, whose address shows a page that asks again until the + * file is ready, then downloads it (the analysis report, pdf-export spec 002). + */ + readonly opens = input(false); readonly pressed = output(); onClick(event: MouseEvent) { diff --git a/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.html b/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.html index 41101fbd1..194e2ecc2 100644 --- a/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.html +++ b/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.html @@ -120,6 +120,7 @@

Analysis Results

[url]="item.url()" [matTooltip]="item.description" [download]="true" + [opens]="item.opens ?? false" [icon]="item.icon" [label]="item.title" > diff --git a/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.ts b/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.ts index 4f602db23..8ddbd4a41 100644 --- a/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.ts +++ b/projects/pathway-browser/src/app/details/tabs/download-tab/download-tab.component.ts @@ -66,6 +66,8 @@ type AnaLysisItem = { url: Signal; icon: Icon; isShown: Signal; + /** Opened in a new tab rather than downloaded (see DownloadButtonComponent). */ + opens?: boolean; }; type DiagramItem = { @@ -276,7 +278,11 @@ export class DownloadTabComponent { { title: 'PDF Result', description: - 'Download a detailed report with the most significant pathway analysis results in PDF format', + 'Open a detailed report with the most significant pathway analysis results in PDF format, in a new tab: it is made in the background, then downloads', + // Made in the background by pdf-export (its spec 002): the address + // answers a page that asks again until the report is ready. Fetched and + // saved here, that page would be saved as the report. + opens: true, url: computed(() => analysisReportUrl( ANALYSIS_SERVICE, diff --git a/projects/pathway-browser/src/app/services/managed-download.directive.ts b/projects/pathway-browser/src/app/services/managed-download.directive.ts index 28635e977..f02713386 100644 --- a/projects/pathway-browser/src/app/services/managed-download.directive.ts +++ b/projects/pathway-browser/src/app/services/managed-download.directive.ts @@ -1,4 +1,12 @@ -import { Directive, ElementRef, computed, inject, signal } from '@angular/core'; +import { + Directive, + ElementRef, + booleanAttribute, + computed, + inject, + input, + signal, +} from '@angular/core'; import { DownloadPhase, FileDownloadService, @@ -41,6 +49,14 @@ export class ManagedDownloadDirective { private anchor = inject>(ElementRef); private readonly active = signal(null); + /** + * Whether this link is fetched here. Bound false, the link is the browser's: + * an address that answers a page while its file is made (the analysis + * report, made in the background, pdf-export spec 002) opens in its tab + * rather than being fetched and saved, which would save that page. + */ + readonly managed = input(true, { alias: 'crManagedDownload', transform: booleanAttribute }); + // Deliberately not cancelled when the link goes away. A reader who clicked // download asked for the file, and some of these links live inside a menu // that closes on the click -- cancelling then would have thrown away the @@ -67,6 +83,7 @@ export class ManagedDownloadDirective { }); onClick(event: MouseEvent) { + if (!this.managed()) return; // A modified click is the reader asking the browser to handle it. if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey || event.button !== 0) { return; diff --git a/specs/010-java-services/spec.md b/specs/010-java-services/spec.md index 2f78ac53e..33ede0a0f 100644 --- a/specs/010-java-services/spec.md +++ b/specs/010-java-services/spec.md @@ -174,9 +174,11 @@ A researcher submits an analysis while the service is busy. Their analysis waits - **FR-019**: The shape of an analysis result, as the site reads it, MUST be defined once in this repository and checked against real results by tests. - **FR-021**: Requests that start a computation (submitting identifiers, mapping, importing, a single identifier, a species comparison, and the PDF report) MUST be admitted a limited number at a time, set by configuration. Those beyond the limit MUST wait in arrival order on a waiting list of bounded length, for no longer than a bounded time. + - _Amended 14 Nov 2026 (T066):_ the PDF report is no longer the AnalysisService's or queued here: it is made by pdf-export (its spec 002), to which the gateway sends it unqueued (FR-025). - **FR-022**: A request that cannot be admitted, because the waiting list is full or its wait ran out, MUST be answered at once with a "service unavailable" status, in the service's own error format, saying when to retry. It MUST NOT be run later. - **FR-023**: Every other request (stored results, downloads, metadata) MUST pass through without waiting. Every answer MUST reach the reader byte-identical to the AnalysisService's own, headers included. - **FR-024**: A waiting request whose reader disconnects MUST lose its place and never be run. The limit, the waiting list's length and the longest wait MUST be reported with the queue's current state on a health check. +- **FR-025**: The analysis report PDF (`GET /AnalysisService/report/…`) MUST be sent to pdf-export without waiting, counted in its client's share and refused past it, with the client named in `X-Reactome-Client` by the gateway alone (never a value the request sent); when pdf-export cannot be reached the answer MUST be 503 in the service's error format (pdf-export spec 002, research R6; T066). - **FR-020**: Before the shared application server is switched off, each of its other webapps MUST have a recorded home or retirement, decided on measured use. Besides ContentService and AnalysisService it hosts nine: the experiment digester, the search no-result report service, the old diagram and fireworks widgets, the old Pathway Browser, popular pathways, overlays, the old RESTful API and the root application (research R10). ### Key Entities diff --git a/specs/010-java-services/tasks.md b/specs/010-java-services/tasks.md index 192db87fe..9d35869d3 100644 --- a/specs/010-java-services/tasks.md +++ b/specs/010-java-services/tasks.md @@ -224,6 +224,7 @@ Each cutover is one PR. It adds the group's patterns to node's table, one nginx - [x] T063 [US6] Have the site say "the service is busy, trying again" on a 503 with Retry-After, and retry once after the time it names. Test it in the site's analysis service, failing first. — #539 - [x] T064 [US6] Review, CI, merge, deploy to beta. Check with `curl --resolve` that an analysis, a stored result and a download pass through unchanged, and that `/health` shows the queue. — #539, deployed 19 Oct 2026 after two reviews: an analysis took a lane place; a stored result, its not-found list and both downloads (CSV and JSON) were byte-identical through the gateway and direct, and `/health` showed the lanes and the counts (1 admitted, 4 passed). Ticked in #540, after the merge. - [x] T065 [US6] A share per client (review of 20 Oct): with places scarce, one address keeping a lane and its waiting list full had everyone else refused. A client -- nginx's X-Real-IP, an IPv6 one by its /64 -- has at most `PER_CLIENT` (8) computations running or waiting; beyond that, one that would have to wait is refused, while one that finds a place free still runs, so a class behind one address is not refused by an idle service. Each rule shown failing by mutation. — #574 +- [x] T066 [US6] The analysis report PDF to pdf-export (pdf-export spec 002 T018 and T021, research R6): the gateway sends `GET` (and `HEAD`) `/AnalysisService/report/…` to pdf-export, a second upstream (`REPORT_UPSTREAM`), not queued (the report lane and `REPORT_LIMIT` gone); a client's reports in flight count towards `PER_CLIENT`, refused past it, and a reader who leaves gives its place back; pdf-export is told the client in `X-Reactome-Client`, set to the gateway's `clientOf` over anything the request sent; pdf-export unreachable is `503` in the analysis service's JSON form. Everything else still goes to the AnalysisService. nginx sets `X-Reactome-Client` from the client's address on `/AnalysisService/`, and `deploy/nginx/test-config.sh` runs each deployment's image to show a request naming another client is still named by its address. pdf-export pinned to `55cde47` (its T020 replay, T026, T027), its working folder a `tmpfs`, still on loopback. The "PDF Result" button opens the report's address in a new tab. Each rule shown failing; pdf-export's errors through the gateway compared with the AnalysisService's on this host for T001's error cases; `documentation/declared-differences.md` updated. Deployed by the user. — #960 --- diff --git a/tools/analysis-gateway/gateway.spec.ts b/tools/analysis-gateway/gateway.spec.ts index 5f0c776c8..9c8b23b95 100644 --- a/tools/analysis-gateway/gateway.spec.ts +++ b/tools/analysis-gateway/gateway.spec.ts @@ -16,6 +16,11 @@ const headersSeen: string[][] = []; const held: (() => void)[] = []; let upstream: Server; let upstreamUrl: string; +/** What the stand-in pdf-export was sent, and its reports not yet answered. */ +const reportsArrived: { url: string; headers: string[] }[] = []; +const reportsHeld: (() => void)[] = []; +let reports: Server; +let reportsUrl: string; beforeAll(async () => { upstream = createServer((req, res) => { @@ -57,26 +62,45 @@ beforeAll(async () => { ]); res.end(JSON.stringify({ path: req.url, length: body.length })); }; - if (/\/(identifiers|species|report)\//.test(url) && !url.includes('quick')) held.push(finish); + if (/\/(identifiers|species)\//.test(url) && !url.includes('quick')) held.push(finish); else finish(); }); }); await new Promise((resolve) => upstream.listen(0, '127.0.0.1', resolve)); upstreamUrl = `http://127.0.0.1:${(upstream.address() as AddressInfo).port}`; + // A stand-in pdf-export: each report held until the test lets it go, as + // pdf-export holds a request while it makes the report. + reports = createServer((req, res) => { + reportsArrived.push({ url: req.url ?? '', headers: req.rawHeaders }); + req.resume(); + reportsHeld.push(() => { + res.writeHead(200, { 'Content-Type': 'application/pdf' }); + res.end('%PDF-'); + }); + }); + await new Promise((resolve) => reports.listen(0, '127.0.0.1', resolve)); + reportsUrl = `http://127.0.0.1:${(reports.address() as AddressInfo).port}`; +}); +afterAll(async () => { + await new Promise((resolve) => upstream.close(resolve)); + reports.closeAllConnections(); + await new Promise((resolve) => reports.close(resolve)); }); -afterAll(() => new Promise((resolve) => upstream.close(resolve))); beforeEach(() => { seen.length = 0; arrived.length = 0; headersSeen.length = 0; held.length = 0; + reportsArrived.length = 0; + reportsHeld.length = 0; }); /** A gateway on its own port, with small limits, for one test. */ async function start(settings: GatewaySettings = {}) { const server = gateway({ upstream: upstreamUrl, - limits: { analysis: 2, report: 1 }, + reportUpstream: reportsUrl, + limits: { analysis: 2 }, queueLength: 2, maxWaitMs: 2000, retryAfter: 7, @@ -188,24 +212,8 @@ describe('the queue', () => { } }); - it('keeps reports on a lane of their own', async () => { - const { base, close } = await start(); - try { - const report = fetch(`${base}/AnalysisService/report/T/Homo%20sapiens/r.pdf`); - await until(() => held.length === 1); - // Analyses still run while a report holds its own lane. - const analysis = post(base, '/AnalysisService/identifiers/?n=1'); - await until(() => held.length === 2); - held.splice(0).forEach((finish) => finish()); - expect((await report).status).toBe(200); - expect((await analysis).status).toBe(200); - } finally { - await close(); - } - }); - it('never holds back a stored result, a download or the metadata', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 }, queueLength: 0 }); + const { base, close } = await start({ limits: { analysis: 1 }, queueLength: 0 }); try { const running = post(base, '/AnalysisService/identifiers/?n=1'); await until(() => held.length === 1); @@ -223,7 +231,7 @@ describe('the queue', () => { }); it('releases the place of a reader who leaves while waiting, and never runs their request', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 } }); + const { base, close } = await start({ limits: { analysis: 1 } }); try { const running = post(base, '/AnalysisService/identifiers/?n=1'); await until(() => held.length === 1); @@ -321,10 +329,8 @@ describe('passing through', () => { try { expect(await health(base)).toMatchObject({ ok: true, - lanes: { - analysis: { limit: 2, active: 0, waiting: 0 }, - report: { limit: 1, active: 0, waiting: 0 }, - }, + lanes: { analysis: { limit: 2, active: 0, waiting: 0 } }, + reports: { upstream: reportsUrl, active: 0 }, queueLength: 2, maxWaitMs: 2000, }); @@ -336,7 +342,7 @@ describe('passing through', () => { describe('the review of #539', () => { it('admits those waiting in the order they came', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 }, queueLength: 5 }); + const { base, close } = await start({ limits: { analysis: 1 }, queueLength: 5 }); try { const answers = [1, 2, 3, 4].map((n) => post(base, `/AnalysisService/identifiers/?n=${n}`)); for (let turn = 1; turn <= 4; turn++) { @@ -367,7 +373,7 @@ describe('the review of #539', () => { }); it('holds a place until the service finishes, even when the reader left mid-analysis', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 } }); + const { base, close } = await start({ limits: { analysis: 1 } }); try { const gone = new AbortController(); const leaver = fetch(`${base}/AnalysisService/identifiers/?n=1`, { @@ -392,7 +398,7 @@ describe('the review of #539', () => { }); it('sees a reader with a large upload leave while waiting, and never runs it', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 } }); + const { base, close } = await start({ limits: { analysis: 1 } }); try { const running = post(base, '/AnalysisService/identifiers/?n=1'); await until(() => held.length === 1); @@ -417,7 +423,7 @@ describe('the review of #539', () => { }); it('sends a large upload whole once it is admitted from the waiting list', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 } }); + const { base, close } = await start({ limits: { analysis: 1 } }); try { const running = post(base, '/AnalysisService/identifiers/?n=1'); await until(() => held.length === 1); @@ -465,7 +471,7 @@ describe('the review of #539', () => { }); it('does not pass connection-scoped headers on, and keeps a refusal out of every cache', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 }, queueLength: 0 }); + const { base, close } = await start({ limits: { analysis: 1 }, queueLength: 0 }); try { await new Promise((resolve, reject) => { const url = new URL(`${base}/AnalysisService/token/T`); @@ -499,15 +505,17 @@ describe('the review of #539', () => { describe('a reader who leaves mid-answer', () => { it('keeps the place until the service has finished the answer', async () => { - const { base, close } = await start(); + const { base, close } = await start({ limits: { analysis: 1 } }); try { const gone = new AbortController(); - const first = await fetch(`${base}/AnalysisService/report/T/x/trickle.pdf`, { + const first = await fetch(`${base}/AnalysisService/identifiers/trickle`, { + method: 'POST', + body: 'P04637', signal: gone.signal, }); expect(first.status).toBe(200); gone.abort(); - const second = fetch(`${base}/AnalysisService/report/T/x/second.pdf`); + const second = post(base, '/AnalysisService/identifiers/?second'); await new Promise((r) => setTimeout(r, 100)); expect(arrived.some((url) => url.includes('second'))).toBe(false); letGo(); @@ -538,7 +546,7 @@ const health = async (base: string) => (await fetch(`${base}/health`)).json() as describe('the second review of #539', () => { it('gives the place back at once when a reader leaves mid-upload', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 } }); + const { base, close } = await start({ limits: { analysis: 1 } }); try { const { call } = upload(base, '/AnalysisService/identifiers/?n=1', { 'Content-Length': '1000', @@ -557,7 +565,7 @@ describe('the second review of #539', () => { }); it('sends the rest of a body still arriving when it is admitted from the waiting list', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 } }); + const { base, close } = await start({ limits: { analysis: 1 } }); try { const running = post(base, '/AnalysisService/identifiers/?n=1'); await until(() => held.length === 1); @@ -580,7 +588,7 @@ describe('the second review of #539', () => { }); it('stops reading at its budget, sends the body whole, and holds nothing once admitted', async () => { - const { base, close } = await start({ limits: { analysis: 1, report: 1 }, waitingBytes: 1000 }); + const { base, close } = await start({ limits: { analysis: 1 }, waitingBytes: 1000 }); try { const running = post(base, '/AnalysisService/identifiers/?n=1'); await until(() => held.length === 1); @@ -606,7 +614,7 @@ describe('the second review of #539', () => { it('gives the place back when the service cannot be reached', async () => { const { base, close } = await start({ upstream: 'http://127.0.0.1:9', - limits: { analysis: 1, report: 1 }, + limits: { analysis: 1 }, maxWaitMs: 500, }); try { @@ -618,10 +626,10 @@ describe('the second review of #539', () => { }); it('gives the place back when the service fails mid-answer', async () => { - const { base, close } = await start({ maxWaitMs: 500 }); + const { base, close } = await start({ limits: { analysis: 1 }, maxWaitMs: 500 }); try { for (const n of [1, 2]) { - const outcome = await fetch(`${base}/AnalysisService/report/T/x/broken${n}.pdf`) + const outcome = await post(base, `/AnalysisService/identifiers/broken${n}`) .then((r) => r.text()) .then( () => 'whole', @@ -636,9 +644,13 @@ describe('the second review of #539', () => { }); it('gives up a call the service has gone silent on, and frees its place', async () => { - const { base, close } = await start({ upstreamTimeoutMs: 200, maxWaitMs: 2000 }); + const { base, close } = await start({ + limits: { analysis: 1 }, + upstreamTimeoutMs: 200, + maxWaitMs: 2000, + }); try { - const hung = fetch(`${base}/AnalysisService/report/T/x/trickle.pdf`) + const hung = post(base, '/AnalysisService/identifiers/trickle') .then((r) => r.text()) .then( () => 'whole', @@ -646,7 +658,7 @@ describe('the second review of #539', () => { ); expect(await hung).toBe('cut'); held.length = 0; - const next = fetch(`${base}/AnalysisService/report/T/x/next.pdf`); + const next = post(base, '/AnalysisService/identifiers/?next'); await until(() => held.length === 1); letGo(); expect((await next).status).toBe(200); @@ -702,7 +714,7 @@ describe('the review of 20 Oct', () => { it('refuses a client beyond its share when it would wait, while others still get in', async () => { const { base, close } = await start({ - limits: { analysis: 2, report: 1 }, + limits: { analysis: 2 }, queueLength: 10, perClient: 2, // Long enough that only the share can refuse the third, never its wait. @@ -741,7 +753,7 @@ describe('the review of 20 Oct', () => { it('runs a client beyond its share while places are free: the share is for scarce places', async () => { const { base, close } = await start({ - limits: { analysis: 3, report: 1 }, + limits: { analysis: 3 }, perClient: 1, }); try { @@ -757,7 +769,7 @@ describe('the review of 20 Oct', () => { it('gives a waiting client its place back when it times out', async () => { const { base, close } = await start({ - limits: { analysis: 1, report: 1 }, + limits: { analysis: 1 }, queueLength: 10, perClient: 2, maxWaitMs: 200, @@ -777,7 +789,7 @@ describe('the review of 20 Oct', () => { it('gives a waiting client its place back when it leaves', async () => { const { base, close } = await start({ - limits: { analysis: 1, report: 1 }, + limits: { analysis: 1 }, queueLength: 10, perClient: 2, maxWaitMs: 30_000, @@ -802,3 +814,206 @@ describe('the review of 20 Oct', () => { } }); }); + +describe('the analysis report, sent to pdf-export (pdf-export spec 002, T021)', () => { + const REPORT = '/AnalysisService/report/T/Homo%20sapiens/report.pdf'; + const report = (base: string, headers: Record = {}, signal?: AbortSignal) => + fetch(`${base}${REPORT}`, { headers, ...(signal && { signal }) }); + /** The values of a header pdf-export was sent, however its name was spelled. */ + const sent = (raw: string[], name: string) => + raw.filter((_, i) => i % 2 === 1 && raw[i - 1]?.toLowerCase() === name); + const letReportGo = () => { + const finish = reportsHeld.shift(); + if (!finish) throw new Error('pdf-export holds no report to let go'); + finish(); + }; + + it('goes to pdf-export, and never to the analysis service', async () => { + const { base, close } = await start(); + try { + const answer = report(base); + await until(() => reportsHeld.length === 1); + letReportGo(); + const pdf = await answer; + expect([pdf.status, pdf.headers.get('content-type'), await pdf.text()]).toEqual([ + 200, + 'application/pdf', + '%PDF-', + ]); + expect(reportsArrived.map((r) => r.url)).toEqual([REPORT]); + expect(arrived).toEqual([]); + // A HEAD runs a GET's handler in Spring: never the AnalysisService's + // report, unqueued. + const head = fetch(`${base}${REPORT}`, { method: 'HEAD' }); + await until(() => reportsHeld.length === 1); + letReportGo(); + expect((await head).status).toBe(200); + expect(arrived).toEqual([]); + } finally { + await close(); + } + }); + + it('sends everything else to the analysis service as before', async () => { + const { base, close } = await start(); + try { + for (const path of [ + '/AnalysisService/token/T', + '/AnalysisService/download/T/result.json.gz', + '/AnalysisService/reports/T', + ]) + expect((await fetch(`${base}${path}`)).status).toBe(200); + const posted = await fetch(`${base}${REPORT}`, { method: 'POST', body: 'x' }); + expect(posted.status).toBe(200); + expect(reportsArrived).toEqual([]); + expect(arrived).toEqual([ + '/AnalysisService/token/T', + '/AnalysisService/download/T/result.json.gz', + '/AnalysisService/reports/T', + REPORT, + ]); + } finally { + await close(); + } + }); + + it('is never queued: more reports at once than any lane had, all with pdf-export', async () => { + const { base, close } = await start({ limits: { analysis: 1 }, queueLength: 0 }); + try { + const answers = [1, 2, 3].map((n) => report(base, { 'X-Real-IP': `203.0.113.${n}` })); + await until(() => reportsHeld.length === 3); + expect((await health(base)).reports.active).toBe(3); + // An analysis still runs beside them. + const analysis = post(base, '/AnalysisService/identifiers/?n=1'); + await until(() => held.length === 1); + letGo(); + reportsHeld.splice(0).forEach((finish) => finish()); + expect((await Promise.all([...answers, analysis])).map((r) => r.status)).toEqual([ + 200, 200, 200, 200, + ]); + expect((await health(base)).reports.active).toBe(0); + } finally { + await close(); + } + }); + + it('names the client to pdf-export by its own reading, whatever the client sent', async () => { + const { base, close } = await start(); + try { + const answer = report(base, { + 'X-Real-IP': '2001:db8:0:1::7', + 'X-Reactome-Client': 'someone-else', + 'X-Forwarded-For': '198.51.100.1', + }); + await until(() => reportsHeld.length === 1); + letReportGo(); + await answer; + const headers = reportsArrived[0]?.headers ?? []; + expect(sent(headers, 'x-reactome-client')).toEqual(['2001:db8:0:1::/64']); + } finally { + await close(); + } + }); + + it('counts a client by its address, not by the name it sent', async () => { + const { base, close } = await start({ perClient: 2 }); + try { + // One address, naming itself differently each time: still one client. + const two = [1, 2].map((n) => + report(base, { 'X-Real-IP': '203.0.113.1', 'X-Reactome-Client': `name-${n}` }) + ); + await until(() => reportsHeld.length === 2); + const third = await report(base, { + 'X-Real-IP': '203.0.113.1', + 'X-Reactome-Client': 'name-3', + }); + expect([third.status, third.headers.get('retry-after')]).toEqual([503, '7']); + expect(reportsArrived.length).toBe(2); + reportsHeld.splice(0).forEach((finish) => finish()); + expect((await Promise.all(two)).map((r) => r.status)).toEqual([200, 200]); + } finally { + await close(); + } + }); + + it('keeps the share per client across analyses and reports, refusing a report past it', async () => { + const { base, close } = await start({ limits: { analysis: 1 }, perClient: 2 }); + try { + const who = { 'X-Real-IP': '203.0.113.1' }; + const analysis = fetch(`${base}/AnalysisService/identifiers/?n=1`, { + method: 'POST', + body: 'P04637', + headers: who, + }); + await until(() => held.length === 1); + const first = report(base, who); + await until(() => reportsHeld.length === 1); + // Two in flight: a second report from this client is refused at once. + const refused = await report(base, who); + expect(refused.status).toBe(503); + // Another client is not. + const other = report(base, { 'X-Real-IP': '203.0.113.2' }); + await until(() => reportsHeld.length === 2); + letGo(); + reportsHeld.splice(0).forEach((finish) => finish()); + expect((await Promise.all([analysis, first, other])).map((r) => r.status)).toEqual([ + 200, 200, 200, + ]); + expect((await health(base)).clients).toBe(0); + } finally { + await close(); + } + }); + + it('gives the client its place back when it leaves a report it is waiting on', async () => { + const { base, close } = await start({ perClient: 1 }); + try { + const who = { 'X-Real-IP': '203.0.113.1' }; + const leaving = new AbortController(); + const left = report(base, who, leaving.signal).catch(() => undefined); + await until(() => reportsHeld.length === 1); + leaving.abort(); + await left; + for (let i = 0; i < 200 && (await health(base)).clients > 0; i++) + await new Promise((r) => setTimeout(r, 10)); + expect((await health(base)).reports.active).toBe(0); + // Its place is free again: not refused for one it no longer waits on. + const again = report(base, who); + await until(() => reportsHeld.length === 2); + reportsHeld.splice(0).forEach((finish) => finish()); + expect((await again).status).toBe(200); + } finally { + await close(); + } + }); + + it("answers 503 in the analysis service's form when pdf-export cannot be reached", async () => { + const { base, close } = await start({ reportUpstream: 'http://127.0.0.1:9' }); + try { + for (let n = 0; n < 2; n++) { + const answer = await report(base, { 'X-Real-IP': '203.0.113.1' }); + expect({ + status: answer.status, + type: answer.headers.get('content-type'), + retry: answer.headers.get('retry-after'), + cache: answer.headers.get('cache-control'), + origin: answer.headers.get('access-control-allow-origin'), + body: await answer.text(), + }).toEqual({ + status: 503, + type: 'application/json;charset=UTF-8', + retry: '7', + cache: 'no-store', + origin: '*', + body: '{"code":503,"reason":"Service Unavailable","messages":["The report service cannot be reached; try again in 7 seconds"]}\n', + }); + } + // Its place given back each time. + expect((await health(base)).clients).toBe(0); + // The analysis service is still reached as before. + expect((await fetch(`${base}/AnalysisService/token/T`)).status).toBe(200); + } finally { + await close(); + } + }); +}); diff --git a/tools/analysis-gateway/gateway.ts b/tools/analysis-gateway/gateway.ts index e4f5316db..b4bee246a 100644 --- a/tools/analysis-gateway/gateway.ts +++ b/tools/analysis-gateway/gateway.ts @@ -14,14 +14,24 @@ * only decides when a request reaches it, and hands back its answer as it * came: status, headers in their order and case, and body. * + * The analysis report PDF, `GET /AnalysisService/report/…`, is not the + * AnalysisService's any more: it goes to pdf-export (its spec 002, research + * R6), a second upstream, and is not queued. pdf-export makes a report in the + * background and holds each request up to 30 s, so a lane of one would make + * every browser asking again wait for the others; its own limits protect it. + * A client's reports in flight still count towards its share (PER_CLIENT), + * refused past it, and pdf-export is told who the client is in + * X-Reactome-Client, always this gateway's own reading of it. + * * ANALYSIS_UPSTREAM the AnalysisService's origin, default http://127.0.0.1:8080 + * REPORT_UPSTREAM pdf-export's origin, default http://127.0.0.1:4370 * GATEWAY_PORT default 4350; GATEWAY_HOST default 127.0.0.1 * ANALYSIS_LIMIT analyses at once, default 4 (below analysis-core's pool of 5) - * REPORT_LIMIT PDF reports at once, default 1 - * QUEUE_LENGTH waiting per lane before refusing, default 40 - * PER_CLIENT one client's computations running or waiting at once, in - * all lanes, default 8: beyond it, one that would have to - * wait is refused (with places free, it still runs) + * QUEUE_LENGTH analyses waiting before refusing, default 40 + * PER_CLIENT one client's analyses running or waiting, and reports in + * flight, at once, default 8: beyond it, an analysis that + * would have to wait is refused (with places free, it still + * runs), and a report is refused * MAX_WAIT_MS longest wait, default 60000 (wait + run inside the edge's 100 s) * RETRY_AFTER seconds a refused reader is told to wait, default 10 * WAITING_BYTES bodies held while waiting, in all, default 268435456 (256 MB) @@ -37,7 +47,10 @@ import { } from 'node:http'; import { fileURLToPath } from 'node:url'; -/** The queues: computations, and the PDF reports, each with its own limit. */ +/** + * What a request is: a computation, which waits its turn here, or a PDF report, + * which goes to pdf-export without waiting. + */ type LaneName = 'analysis' | 'report'; /** What was read of a waiting request's body before its turn came. */ @@ -65,11 +78,13 @@ interface Lane { waiting: Waiting[]; } -/** What /health answers: each lane's state, the settings, and the counts so far. */ +/** What /health answers: the queue's state, the settings, and the counts so far. */ export interface Health { ok: true; upstream: string; - lanes: Record; + lanes: { analysis: { limit: number; active: number; waiting: number } }; + /** pdf-export, and the reports passed to it now. */ + reports: { upstream: string; active: number }; queueLength: number; maxWaitMs: number; perClient: number; @@ -86,7 +101,9 @@ export interface Health { export interface GatewaySettings { upstream?: string; - limits?: Record; + /** pdf-export's origin, where the reports go. */ + reportUpstream?: string; + limits?: { analysis: number }; queueLength?: number; maxWaitMs?: number; perClient?: number; @@ -144,15 +161,24 @@ export function classify( const HOP = new Set(['connection', 'keep-alive', 'proxy-connection', 'te', 'trailer', 'upgrade']); /** Raw headers, name and value alternating, without the hop-by-hop ones. */ -const without = (raw: string[]): string[] => { +const without = (raw: string[], also: ReadonlySet = new Set()): string[] => { const kept: string[] = []; for (let i = 0; i + 1 < raw.length; i += 2) { const name = raw[i] as string; - if (!HOP.has(name.toLowerCase())) kept.push(name, raw[i + 1] as string); + const lower = name.toLowerCase(); + if (!HOP.has(lower) && !also.has(lower)) kept.push(name, raw[i + 1] as string); } return kept; }; +/** + * The header naming the client to pdf-export, whose share of the waiting + * reports trusts it. Always this gateway's, whatever the request sent: one a + * client could choose would let it dodge that share by naming another. + */ +const CLIENT_HEADER = 'X-Reactome-Client'; +const CLIENT_NAMED = new Set([CLIENT_HEADER.toLowerCase()]); + /** * Who a request is from, for the per-client share: the address nginx saw * (X-Real-IP, which it sets from the connection; the gateway listens on @@ -175,6 +201,28 @@ export function clientOf(req: IncomingMessage): string { .join(':')}::/64`; } +/** + * pdf-export cannot be reached: 503 in the analysis service's form, as + * pdf-export writes its own 503 (its spec 002, FR-203), so the report's + * address keeps one error format; not the 502 for the AnalysisService. + */ +function reportUnreachable(res: ServerResponse, retryAfter: number): void { + if (res.headersSent || res.destroyed) return; + const body = `${JSON.stringify({ + code: 503, + reason: 'Service Unavailable', + messages: [`The report service cannot be reached; try again in ${retryAfter} seconds`], + })}\n`; + res.writeHead(503, { + 'Access-Control-Allow-Origin': '*', + 'Content-Type': 'application/json;charset=UTF-8', + 'Content-Length': String(Buffer.byteLength(body)), + 'Retry-After': String(retryAfter), + 'Cache-Control': 'no-store', + }); + res.end(body); +} + /** The service's own error envelope, as it writes a 404 or a 410. */ function busy(res: ServerResponse, retryAfter: number): void { if (res.headersSent || res.destroyed) return; @@ -193,7 +241,8 @@ function busy(res: ServerResponse, retryAfter: number): void { export function gateway({ upstream = 'http://127.0.0.1:8080', - limits = { analysis: 4, report: 1 }, + reportUpstream = 'http://127.0.0.1:4370', + limits = { analysis: 4 }, queueLength = 40, maxWaitMs = 60_000, perClient = 8, @@ -202,10 +251,10 @@ export function gateway({ upstreamTimeoutMs = 300_000, }: GatewaySettings = {}) { const target = new URL(upstream); - const lanes: Record = { - analysis: { limit: limits.analysis, active: 0, waiting: [] }, - report: { limit: limits.report, active: 0, waiting: [] }, - }; + const reportTarget = new URL(reportUpstream); + const analysis: Lane = { limit: limits.analysis, active: 0, waiting: [] }; + /** Reports passed to pdf-export and not yet answered. */ + let reports = 0; const counts = { passed: 0, admitted: 0, waited: 0, refused: 0, left: 0, failed: 0 }; let held = 0; /** Each client's computations running or waiting, in all lanes. */ @@ -226,12 +275,17 @@ export function gateway({ /** * Send the request on, and its answer back as it came; `done` once the * service has finished with it. `body` is what was read while it waited, - * if anything: the rest still comes from `req`. + * if anything: the rest still comes from `req`. `report` sends it to + * pdf-export instead, naming the client. */ function pass( req: IncomingMessage, res: ServerResponse, - { done = () => {}, body }: { done?: () => void; body?: HeldBody } = {} + { + done = () => {}, + body, + report, + }: { done?: () => void; body?: HeldBody; report?: { client: string } } = {} ): void { let over = false; const finish = () => { @@ -239,13 +293,16 @@ export function gateway({ over = true; done(); }; + const to = report ? reportTarget : target; const call = httpRequest( { - host: target.hostname, - port: target.port, + host: to.hostname, + port: to.port, method: req.method, path: req.url, - headers: without(req.rawHeaders), + headers: report + ? [...without(req.rawHeaders, CLIENT_NAMED), CLIENT_HEADER, report.client] + : without(req.rawHeaders), }, (answer) => { // Over only when the service is: a reader who left mid-answer does not @@ -273,9 +330,19 @@ export function gateway({ }); } ); + /** A report's reader who left before its answer was whole. */ + let abandoned = false; call.on('error', () => { + if (abandoned) { + counts.left++; + finish(); + return; + } counts.failed++; - if (!res.headersSent && !res.destroyed) { + if (report) { + if (!res.headersSent && !res.destroyed) reportUnreachable(res, retryAfter); + else res.destroy(); + } else if (!res.headersSent && !res.destroyed) { res.writeHead(502, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' }); res.end( JSON.stringify({ @@ -287,6 +354,18 @@ export function gateway({ } else res.destroy(); finish(); }); + if (report) { + // A reader who leaves a report: pdf-export goes on making it for the + // next request (its research R7), so only the wait ends here, and with + // it the client's place, rather than holding both for the 30 s + // pdf-export would hold a request nobody reads. + res.on('close', () => { + if (res.writableFinished) return; + abandoned = true; + call.destroy(); + }); + call.on('close', finish); + } // A service that says nothing for as long as nginx waits has lost the // reader already; without this a hung call held its place until Tomcat // restarted, and four of them closed the lane. @@ -399,7 +478,8 @@ export function gateway({ const health: Health = { ok: true, upstream: target.origin, - lanes: { analysis: state(lanes.analysis), report: state(lanes.report) }, + lanes: { analysis: state(analysis) }, + reports: { upstream: reportTarget.origin, active: reports }, queueLength, maxWaitMs, perClient, @@ -411,7 +491,29 @@ export function gateway({ return; } const name = classify(req.method, req.url); - const lane = name && lanes[name]; + if (name === 'report') { + // Never queued: pdf-export holds the request while it makes the + // report, and has its own limits. The share still counts it, and a + // client past its share is refused rather than made to wait. + const client = clientOf(req); + if ((inside.get(client) ?? 0) >= perClient) { + counts.refused++; + busy(res, retryAfter); + return; + } + const out = enter(client); + reports++; + counts.admitted++; + pass(req, res, { + report: { client }, + done: () => { + out(); + reports--; + }, + }); + return; + } + const lane = name === 'analysis' ? analysis : undefined; if (!lane) { counts.passed++; pass(req, res); @@ -458,7 +560,8 @@ if (startedDirectly()) { const host = env['GATEWAY_HOST'] || '127.0.0.1'; const server = gateway({ upstream: env['ANALYSIS_UPSTREAM'] || 'http://127.0.0.1:8080', - limits: { analysis: number('ANALYSIS_LIMIT', 4), report: number('REPORT_LIMIT', 1) }, + reportUpstream: env['REPORT_UPSTREAM'] || 'http://127.0.0.1:4370', + limits: { analysis: number('ANALYSIS_LIMIT', 4) }, queueLength: number('QUEUE_LENGTH', 40), maxWaitMs: number('MAX_WAIT_MS', 60_000), perClient: number('PER_CLIENT', 8),