diff --git a/src/store/dicom-web/dicom-meta-store.ts b/src/store/dicom-web/dicom-meta-store.ts index e84bcd13c..434aa3a58 100644 --- a/src/store/dicom-web/dicom-meta-store.ts +++ b/src/store/dicom-web/dicom-meta-store.ts @@ -49,6 +49,12 @@ interface State { studyPatient: Record; } +// The index has no slot for a series without its study, or an instance +// without its series. +const hasParentUids = (info: Instance) => + (!info.SeriesInstanceUID || !!info.StudyInstanceUID) && + (!info.SopInstanceUID || !!info.SeriesInstanceUID); + export const useDicomMetaStore = defineStore('dicom-meta', { state: (): State => ({ patientInfo: {}, @@ -64,7 +70,8 @@ export const useDicomMetaStore = defineStore('dicom-meta', { needsRebuild: {}, }), actions: { - async importMeta(info: Instance) { + importMeta(info: Instance) { + if (!hasParentUids(info)) return; const patient = { PatientID: info.PatientID || ANONYMOUS_PATIENT_ID, PatientName: info.PatientName || ANONYMOUS_PATIENT, diff --git a/src/store/dicom-web/dicom-web-store.ts b/src/store/dicom-web/dicom-web-store.ts index 0d8ada899..019af0d58 100644 --- a/src/store/dicom-web/dicom-web-store.ts +++ b/src/store/dicom-web/dicom-web-store.ts @@ -19,6 +19,7 @@ import { retrieveStudyMetadata, retrieveSeriesMetadata, parseUrl, + Instance, } from '@/src/core/dicom-web-api'; import { useViewStore } from '@/src/store/views'; @@ -57,6 +58,20 @@ const levelToMetaKey = { type InitialDicomListFetchProgress = 'Idle' | 'Pending' | 'Done'; +// One failed request should not hide what the other requests returned. +const importSettled = (results: PromiseSettledResult[]) => { + const dicoms = useDicomMetaStore(); + results.forEach((result) => { + if (result.status === 'fulfilled') + result.value.forEach((instance) => dicoms.importMeta(instance)); + }); + const failure = results.find((result) => result.status === 'rejected'); + if (failure) + useMessageStore().addError('Failed to fetch DICOMWeb metadata', { + error: failure.reason, + }); +}; + /** * Collect DICOM data from DICOMWeb */ @@ -111,7 +126,7 @@ export const useDicomWebStore = defineStore('dicom-web', () => { const fetchPatientMeta = async (patientKey: string) => { const dicoms = useDicomMetaStore(); - const series = await Promise.all( + const series = await Promise.allSettled( dicoms.patientStudies[patientKey] .map((studyKey) => dicoms.studyInfo[studyKey]) .map(async (studyMeta) => { @@ -124,12 +139,12 @@ export const useDicomWebStore = defineStore('dicom-web', () => { })); }) ); - series.flat().forEach((instance) => dicoms.importMeta(instance)); + importSettled(series); }; const fetchVolumesMeta = async (volumeKeys: string[]) => { const dicoms = useDicomMetaStore(); - const series = await Promise.all( + const series = await Promise.allSettled( volumeKeys.map(async (volumeKey) => { const volumeMeta = dicoms.volumeInfo[volumeKey]; const studyMeta = dicoms.studyInfo[dicoms.volumeStudy[volumeKey]]; @@ -144,7 +159,7 @@ export const useDicomWebStore = defineStore('dicom-web', () => { })); }) ); - series.flat().forEach((instance) => dicoms.importMeta(instance)); + importSettled(series); }; const volumes = ref({} as Progress); @@ -161,12 +176,6 @@ export const useDicomWebStore = defineStore('dicom-web', () => { total: 0, }; - const { SeriesInstanceUID: seriesInstanceUID } = - dicoms.volumeInfo[volumeKey]; - const studyKey = dicoms.volumeStudy[volumeKey]; - const { StudyInstanceUID: studyInstanceUID } = dicoms.studyInfo[studyKey]; - const seriesInfo = { studyInstanceUID, seriesInstanceUID }; - const progressCallback = ({ loaded, total }: ProgressEvent) => { volumes.value[volumeKey] = { ...volumes.value[volumeKey], @@ -176,6 +185,15 @@ export const useDicomWebStore = defineStore('dicom-web', () => { }; try { + const volumeInfo = dicoms.volumeInfo[volumeKey]; + if (!volumeInfo) { + throw new Error('DICOMWeb metadata is unavailable for this series'); + } + const studyInfo = dicoms.studyInfo[dicoms.volumeStudy[volumeKey]]; + const seriesInfo = { + studyInstanceUID: studyInfo.StudyInstanceUID, + seriesInstanceUID: volumeInfo.SeriesInstanceUID, + }; const files = await fetchSeries( cleanHost.value, seriesInfo, @@ -247,16 +265,16 @@ export const useDicomWebStore = defineStore('dicom-web', () => { ...fetchedMeta, })); fullMeta.forEach((instance) => dicoms.importMeta(instance)); + + // downloadVolume reports its own failures, so the list need not wait. + if (deepestLevel === 'series' && fullMeta.length > 0) { + downloadVolume(Object.values(parsedURL.value).pop() as string); + } } catch (error) { fetchError.value = error; console.error(error); } - if (deepestLevel === 'series') { - const seriesID = Object.values(parsedURL.value).pop() as string; - downloadVolume(seriesID); - } - fetchDicomsProgress.value = 'Done'; }; diff --git a/tests/pageobjects/volview.page.ts b/tests/pageobjects/volview.page.ts index a43763fea..71ba814e3 100644 --- a/tests/pageobjects/volview.page.ts +++ b/tests/pageobjects/volview.page.ts @@ -92,6 +92,15 @@ class VolViewPage extends Page { ); } + async getNotificationTitles() { + await this.notifications.click(); + const titles = $$( + '.message-center .v-expansion-panel-title .header > span' + ); + await titles[0].waitForDisplayed(); + return titles.map((title) => title.getText()); + } + async selectTool(icon: string) { const button = $(`button.tool-btn i[class~=${icon}]`); await button.waitForClickable(); diff --git a/tests/server/dicomweb-server.ts b/tests/server/dicomweb-server.ts new file mode 100644 index 000000000..251855737 --- /dev/null +++ b/tests/server/dicomweb-server.ts @@ -0,0 +1,86 @@ +import express from 'express'; +import cors from 'cors'; +import type { Server } from 'http'; +import type { AddressInfo } from 'net'; + +export type Reply = { + status: number; + json?: unknown; + dicoms?: Uint8Array[]; +}; + +export type DicomWebReplies = { + studySearch: Reply; + seriesSearch: Reply; + seriesMetadata: Reply; + seriesRetrieval: Reply; +}; + +const BOUNDARY = 'volview-e2e-boundary'; + +// WADO-RS answers a series retrieval with one multipart/related part per instance. +const multipartRelated = (dicoms: Uint8Array[]) => + Buffer.concat([ + ...dicoms.flatMap((dicom) => [ + Buffer.from(`--${BOUNDARY}\r\nContent-Type: application/dicom\r\n\r\n`), + Buffer.from(dicom), + Buffer.from('\r\n'), + ]), + Buffer.from(`--${BOUNDARY}--`), + ]); + +const send = (res: express.Response, reply: Reply) => { + if (reply.dicoms) { + res + .status(reply.status) + .set( + 'Content-Type', + `multipart/related; type="application/dicom"; boundary=${BOUNDARY}` + ) + .send(multipartRelated(reply.dicoms)); + return; + } + res.status(reply.status).json(reply.json ?? null); +}; + +/** + * Serves the DICOMweb routes VolView requests for a linked study or series, + * answering each with whatever the spec last assigned to `replies`. + */ +export async function startDicomWebServer() { + const replies: DicomWebReplies = { + studySearch: { status: 404 }, + seriesSearch: { status: 404 }, + seriesMetadata: { status: 404 }, + seriesRetrieval: { status: 404 }, + }; + + const app = express(); + app.use(cors()); + app.get('/dicom-web/studies', (_req, res) => send(res, replies.studySearch)); + app.get('/dicom-web/studies/:study/series', (_req, res) => + send(res, replies.seriesSearch) + ); + app.get('/dicom-web/studies/:study/series/:series/metadata', (_req, res) => + send(res, replies.seriesMetadata) + ); + app.get('/dicom-web/studies/:study/series/:series', (_req, res) => + send(res, replies.seriesRetrieval) + ); + + // Port 0 so specs in parallel workers never contend for the shared AUX_PORT. + const server = await new Promise((resolve) => { + const listening = app.listen(0, () => resolve(listening)); + }); + const { port } = server.address() as AddressInfo; + + return { + replies, + root: `http://localhost:${port}/dicom-web`, + stop: () => + new Promise((resolve) => { + server.closeAllConnections(); + server.close(() => resolve()); + }), + }; +} diff --git a/tests/specs/dicomweb-metadata-failures.e2e.ts b/tests/specs/dicomweb-metadata-failures.e2e.ts new file mode 100644 index 000000000..62470a0f6 --- /dev/null +++ b/tests/specs/dicomweb-metadata-failures.e2e.ts @@ -0,0 +1,143 @@ +import { volViewPage } from '../pageobjects/volview.page'; +import { startDicomWebServer } from '../server/dicomweb-server'; +import { buildSyntheticDicom, newUid } from './syntheticDicom'; + +const SLICE_COUNT = 3; + +const tag = (vr: string, value: unknown) => ({ vr, Value: [value] }); + +// A QIDO-RS series search result in the DICOM JSON model. +const seriesRecord = (studyUid: string, seriesUid: string) => ({ + '00100010': tag('PN', { Alphabetic: 'DICOMWeb^Patient' }), + '00100020': tag('LO', 'dicomweb-patient'), + '0020000D': tag('UI', studyUid), + '0020000E': tag('UI', seriesUid), + '00080060': tag('CS', 'MR'), + '0008103E': tag('LO', 'Linked series'), +}); + +const makeSeries = (studyUid = newUid()) => { + const seriesUid = newUid(); + const sopUids = Array.from({ length: SLICE_COUNT }, () => newUid()); + return { + studyUid, + seriesUid, + search: [seriesRecord(studyUid, seriesUid)], + metadata: sopUids.map((sopUid, i) => ({ + ...seriesRecord(studyUid, seriesUid), + '00080018': tag('UI', sopUid), + '00200013': tag('IS', String(i + 1)), + '00280010': tag('US', 4), + '00280011': tag('US', 4), + })), + dicoms: sopUids.map((sopUid, i) => + buildSyntheticDicom({ + studyUid, + seriesUid, + sopUid, + instanceNumber: i + 1, + imageOrientationPatient: [1, 0, 0, 0, 1, 0], + imagePositionPatient: [0, 0, i], + }) + ), + }; +}; + +const dicomWebMessage = () => $('#data-module p.error-message'); + +describe('DICOMWeb metadata failures', () => { + let server: Awaited>; + + const linkTo = (...path: string[]) => + volViewPage.open(`?dicomweb=${[server.root, ...path].join('/')}`); + + const linkToSeries = ({ + studyUid, + seriesUid, + }: ReturnType) => + linkTo('studies', studyUid, 'series', seriesUid); + + beforeEach(async () => { + server = await startDicomWebServer(); + }); + + afterEach(async () => { + await server.stop(); + }); + + it('loads a linked series once its failed metadata request is retried', async () => { + const series = makeSeries(); + server.replies.seriesMetadata = { status: 500 }; + await linkToSeries(series); + + await expect(dicomWebMessage()).toHaveText( + expect.stringContaining('Error fetching DICOMWeb data') + ); + expect(await volViewPage.getNotificationsCount()).toBe(0); + + server.replies.seriesSearch = { status: 200, json: series.search }; + server.replies.seriesMetadata = { status: 200, json: series.metadata }; + server.replies.seriesRetrieval = { status: 200, dicoms: series.dicoms }; + // Closing Settings unmounts the DICOMWeb fields, which retries a failed fetch. + await volViewPage.openSettings(); + await volViewPage.closeSettings(); + + await $('.volume-card .mdi-check').waitForDisplayed(); + await volViewPage.waitForViews(); + expect(await volViewPage.getNotificationsCount()).toBe(0); + }); + + it('reports a linked series with no metadata without starting a download', async () => { + server.replies.seriesMetadata = { status: 200, json: [] }; + await linkToSeries(makeSeries()); + + await expect(dicomWebMessage()).toHaveText('Found zero dicoms.'); + expect(await volViewPage.getNotificationsCount()).toBe(0); + }); + + it('reports a linked series that its metadata does not describe', async () => { + const linked = makeSeries(); + const other = makeSeries(linked.studyUid); + server.replies.seriesSearch = { status: 200, json: other.search }; + server.replies.seriesMetadata = { status: 200, json: other.metadata }; + await linkToSeries(linked); + + await volViewPage.waitForNotification(); + expect(await volViewPage.getNotificationTitles()).toEqual([ + 'Failed to load DICOM', + ]); + }); + + it('lists the studies of a search that also returns a record without a study UID', async () => { + const { studyUid, seriesUid } = makeSeries(); + server.replies.studySearch = { + status: 200, + json: [ + seriesRecord(studyUid, seriesUid), + { + '00100010': tag('PN', { Alphabetic: 'Malformed^Record' }), + '0020000E': tag('UI', newUid()), + }, + ], + }; + await linkTo(); + + await expect($('#data-module .patient-header-name')).toHaveText( + 'DICOMWeb^Patient' + ); + }); + + it('stops the series spinner when browsing a study whose series metadata fails', async () => { + const series = makeSeries(); + server.replies.seriesSearch = { status: 200, json: series.search }; + server.replies.seriesMetadata = { status: 500 }; + await linkTo('studies', series.studyUid); + + await volViewPage.waitForNotification(); + await expect($('.volume-list')).toExist(); + await expect($('.volume-list .fetching')).not.toExist(); + expect(await volViewPage.getNotificationTitles()).toEqual([ + 'Failed to fetch DICOMWeb metadata', + ]); + }); +}); diff --git a/tests/specs/zero-voxel-spacing.e2e.ts b/tests/specs/zero-voxel-spacing.e2e.ts index 0ffe6347c..504ed19bd 100644 --- a/tests/specs/zero-voxel-spacing.e2e.ts +++ b/tests/specs/zero-voxel-spacing.e2e.ts @@ -1,13 +1,6 @@ import { volViewPage } from '../pageobjects/volview.page'; import { writeManifestToFile, writeMetaImage } from './utils'; -const notificationTitles = async () => { - await volViewPage.notifications.click(); - const titles = $$('.message-center .v-expansion-panel-title .header > span'); - await titles[0].waitForDisplayed(); - return titles.map((title) => title.getText()); -}; - describe('An image with zero voxel spacing', () => { it('can be painted and reports the invalid spacing', async () => { // itk-wasm passes a MetaImage's zero ElementSpacing through unchanged. @@ -26,6 +19,8 @@ describe('An image with zero voxel spacing', () => { const views2D = await volViewPage.getViews2D(); await volViewPage.paintStrokeOnView(views2D[0]); - expect(await notificationTitles()).toEqual(['Invalid voxel spacing']); + expect(await volViewPage.getNotificationTitles()).toEqual([ + 'Invalid voxel spacing', + ]); }); });