feat(retrieve) : Allow OHIF retrieve from multiple servers #3633

rodrigobasilio2022 · 2023-09-06T12:16:37Z

Context

This PR is related to the issue #3507, and allows OHIF to search in a main server and retrieves series from multiple dicomweb servers.

Changes & Results

The main change is this PR is the ability to write a datasource configuration as an array, where one can define multiple servers configurations. The first one will be the main server and OHIF will use this configuration to search for studies. The other configurations will be used to retrieve series, as described in the issue. This PR also shows how to define parameters in a URL.

Testing

To test, one need to setup or use two different dicomweb servers, and prepare and subdivide some studies series between them. After that, define the configuration attribute of a datasource as an array as follow

dataSources: [
{
namespace: '@ohif/extension-default.dataSourcesModule.dicomweb',
sourceName: 'dicomweb',
configuration: [{..}, {..}]
...

Here is one example of a datasource configuration with two dicomservers: one static wado server and other local orthanc server

{
      namespace: '@ohif/extension-default.dataSourcesModule.dicomweb',
      sourceName: 'dicomweb',
      configuration: [
		  {
			friendlyName: 'Static WADO Server',
			name: 'staticwado',

			wadoUriRoot: 'https://d33do7qe4w26qo.cloudfront.net/dicomweb',
			qidoRoot: 'https://d33do7qe4w26qo.cloudfront.net/dicomweb',
			wadoRoot: 'https://d33do7qe4w26qo.cloudfront.net/dicomweb',
			qidoSupportsIncludeField: false,
			supportsReject: false,
			imageRendering: 'wadors',
			thumbnailRendering: 'wadors',
			enableStudyLazyLoad: true,
			supportsFuzzyMatching: false,
			supportsWildcard: true,
			staticWado: true,
			singlepart: 'bulkdata,video',
			bulkDataURI: {
			  enabled: true,
			  relativeResolution: 'studies',
			},
			omitQuotationForMultipartRequest: true,
		  },
		  {
			friendlyName: 'Local Orthanc server',
			name: 'orthanc',

			wadoUriRoot: 'http://localhost/dicom-web',
			qidoRoot: 'http://localhost/dicom-web',
			wadoRoot: 'http://localhost/dicom-web',
			qidoSupportsIncludeField: false,
			supportsReject: false,
			imageRendering: 'wadors',
			thumbnailRendering: 'wadors',
			enableStudyLazyLoad: true,
			supportsFuzzyMatching: false,
			supportsWildcard: true,
			staticWado: true,
			singlepart: 'bulkdata,video',
			bulkDataURI: {
			  enabled: true,
			  relativeResolution: 'studies',
			},
			omitQuotationForMultipartRequest: true,
		  },
	  ]
}

Its important to define in each configuration the attribute that will uniquely identifies the configuration.

In my test, I used the oficial OHIF dicomweb server and a local one, with a TotalSegmentator series produced from a study in the oficial OHIF dicomweb server

Opening the study, both series will appear and the segmentation series could be opened, despite not listed in the main dicomweb server.

To test with the same data as displayed here, one can download this dicom segmentation file
https://drive.google.com/file/d/124RgsQNRp6EmQIie9jr-6m41DFzt-T-K/view?usp=drive_link

and use the link

http://localhost:3000/viewer?StudyInstanceUIDs=2.16.840.1.114362.1.11972228.22789312658.616067305.306.2

To test the URL parameter secondGoogleServer, one needs to setup two google datastores, one with the main series and another with derived series, and configure in the default.js file the main one. The second one should be configured as follows:

        {
          friendlyName: 'Secondary Google Server',
          name: 'secondary server',

          qidoSupportsIncludeField: false,
          supportsReject: false,
          imageRendering: 'wadors',
          thumbnailRendering: 'wadors',
          enableStudyLazyLoad: true,
          supportsFuzzyMatching: false,
          supportsWildcard: false,
          staticWado: true,
          singlepart: 'bulkdata,video',
          bulkDataURI: {
            enabled: true,
            relativeResolution: 'studies',
          },
          omitQuotationForMultipartRequest: true,
          onConfiguration: (dicomWebConfig, options) => {
            function parseGoogleServerParameter(param) {
              // remove first slash
              if (param[0] === '/') {
                param = param.slice(1);
              }
              const tokens = param.split('/');
              const params = {};

              for (let i = 0; i < Math.floor(tokens.length / 2); i++) {
                params[tokens[i * 2]] = tokens[i * 2 + 1];
              }
              return params;
            }

            const { query } = options;
            const secondServer = query.get('secondGoogleServer');
            if (secondServer) {
              const { projects, locations, datasets, dicomStores } =
                parseGoogleServerParameter(secondServer);
              const pathUrl = `https://healthcare.googleapis.com/v1/projects/${projects}/locations/${locations}/datasets/${datasets}/dicomStores/${dicomStores}/dicomWeb`;
              return {
                ...dicomWebConfig,
                wadoRoot: pathUrl,
                qidoRoot: pathUrl,
                wadoUri: pathUrl,
                wadoUriRoot: pathUrl,
              };
            }
          },
        },

Note that the URL parameter is defined not in the OHIF code, but outside, in the default.js.

The following pictures demonstrate this functionality.

Main datastore, with two studies. Note that the expanded study has three series.

Opening the study using an URL with the secondGoogleServer parameter, it shows four series, the dicom seg series belonging only to the second server

Next picture shows that the parameter SeriesInstanceUID works to filter a particular series. Remember that this url parameter works as follows: it tries to filter the series, but if the result is empty, it returns all series found of a particular study. That's why the segmentation series also appears

Checklist

PR

[] My Pull Request title is descriptive, accurate and follows the
semantic-release format and guidelines.

Code

My code has been well-documented (function documentation, inline comments,
etc.)

Public Documentation Updates

[] The documentation page has been updated as necessary for any public API
additions or removals.

Tested Environment

OS: Windows 10
Node version: 16.14.0
Browser: Chrome 116.0.5845.97, Firefox 116.0.3

netlify · 2023-09-06T12:16:43Z

✅ Deploy Preview for ohif-platform-docs ready!

Name	Link
🔨 Latest commit	`6f4f2e6`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/65524e123da33f0008a62e1b
😎 Deploy Preview	https://deploy-preview-3633--ohif-platform-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2023-09-06T12:16:44Z

✅ Deploy Preview for ohif-dev ready!

Name	Link
🔨 Latest commit	`6f4f2e6`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-dev/deploys/65524e124f1e93000839ac4f
😎 Deploy Preview	https://deploy-preview-3633--ohif-dev.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2023-09-06T12:19:22Z

Codecov Report

Merging #3633 (6f4f2e6) into master (8a335bd) will decrease coverage by 0.67%.
Report is 121 commits behind head on master.
The diff coverage is 4.25%.

@@            Coverage Diff             @@
##           master    #3633      +/-   ##
==========================================
- Coverage   46.23%   45.57%   -0.67%     
==========================================
  Files          78       79       +1     
  Lines        1276     1299      +23     
  Branches      312      319       +7     
==========================================
+ Hits          590      592       +2     
- Misses        548      562      +14     
- Partials      138      145       +7

Files	Coverage Δ
platform/app/src/routes/WorkList/filtersMeta.js	`0.00% <ø> (ø)`
platform/core/src/utils/downloadCSVReport.js	`0.00% <ø> (ø)`
platform/core/src/utils/index.js	`100.00% <ø> (ø)`
...form/core/src/utils/isDisplaySetReconstructable.js	`5.00% <ø> (+0.18%)`	⬆️
...ils/metadataProvider/getPixelSpacingInformation.js	`0.00% <0.00%> (ø)`
platform/core/src/log.js	`14.28% <0.00%> (-85.72%)`	⬇️
...services/DicomMetadataStore/createStudyMetadata.js	`0.00% <0.00%> (ø)`
...ervices/DicomMetadataStore/createSeriesMetadata.js	`0.00% <0.00%> (ø)`
platform/core/src/utils/addAccessors.js	`9.09% <9.09%> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 013068b...6f4f2e6. Read the comment docs.

sedghi · 2023-09-07T01:55:40Z

I will review after Igor if that is ok

igoroctaviano

I added some questions to bootstrap the review process!

extensions/cornerstone-dicom-rt/src/loadRTStruct.js

extensions/default/src/DicomWebDataSource/retrieveStudyMetadata.js

extensions/default/src/DicomWebDataSource/utils/clientManager.ts

igoroctaviano · 2023-11-03T15:11:31Z

@rodrigobasilio2022 can you please finish the comments/follow up with @sedghi so we can merge this PR asap? I think you doing it its going to be a lot faster. Thanks in advance!

…into feat/TwoServers

rodrigobasilio2022 · 2023-11-07T17:36:22Z

@sedghi Done the modifications. Please review

…rs into feat/TwoServers

sedghi · 2023-11-13T18:15:53Z

Looks much! better, thanks.
So right now it will look into all the data sources inside the configuration?

Do you think this is good enough? or we should be more specific, that grab from these two data sources. (we have defaultDataSourceName right now, how about we have that as string | string[] and that way we make it more explicit?

@wayfarer3130

igoroctaviano · 2023-11-13T18:25:33Z

Looks much! better, thanks. So right now it will look into all the data sources inside the configuration?

Do you think this is good enough? or we should be more specific, that grab from these two data sources. (we have defaultDataSourceName right now, how about we have that as string | string[] and that way we make it more explicit?

@wayfarer3130

The idea is to allow multiple configurations/instances per data source. Right now its only going to work for dicomweb data source but later we can add support for other kinds. I tink it does not affect the defaultDataSourceName property because that property is in the data sources level, and this change here is to allow multiple configurations/instances per data source.

sedghi · 2023-11-13T18:43:43Z

@igoroctaviano what I mean is that right now (after this PR), if there are 3 DICOM web data sources in the configuration it will try to fetch from all three which might or might not be ideal. @wayfarer3130 what do you think? should we be more explicit or not?

Like right now If I try it it will say, since it is trying each server and I don't have my local servers up and runnign

rodrigobasilio2022 · 2023-11-13T18:58:53Z

@igoroctaviano what I mean is that right now (after this PR), if there are 3 DICOM web data sources in the configuration it will try to fetch from all three which might or might not be ideal. @wayfarer3130 what do you think? should we be more explicit or not?

Like right now If I try it it will say, since it is trying each server and I don't have my local servers up and runnign

But if you dont want to use the three configurations, you have to set it properly. You can define multiple datasources, each one with multiple configurations and you can select the datasource depending on the mode.

wayfarer3130 · 2023-11-13T19:27:12Z

Please could we use the standard DICOM attributes for this:
RetrieveAETitle
There are already DICOMweb compliant systems out there which provide this tag, and I know of at least one DICOMweb viewer system which uses those tags to query the right system.

The basic idea:

Configure a DataSource to follow RetreiveAE titles (simple flag)
Optionally: Configure data source to have default RetrieveAE titles for some parts (study result or metadata or image)
When querying for the series for a study, use the RetrieveAETitle to lookup the data source using the existing name
When querying for metadata, use the RetrieveAE title from the series list to lookup the data source for the metadata
When querying for image data, use the RetrieveAE title from the instance in the metadata to retrieve data

That should be a lot easier to implement than a hierarhical structure, and can easily be replicated for non-DICOMweb type systems, or default values provided as a map or function somewhere.

wayfarer3130 · 2023-11-13T19:27:33Z

extensions/cornerstone-dicom-rt/src/loadRTStruct.js

@@ -39,6 +39,7 @@ async function checkAndLoadContourData(instance, datasource) {
          StudyInstanceUID: instance.StudyInstanceUID,
          SeriesInstanceUID: instance.SeriesInstanceUID,
          SOPInstanceUID: instance.SOPInstanceUID,
+          clientName: instance?.clientName,


RetrieveAETitle

wayfarer3130 · 2023-11-13T19:28:28Z

extensions/default/src/DicomWebDataSource/index.js

 import getDirectURL from '../utils/getDirectURL';
 import { fixBulkDataURI } from './utils/fixBulkDataURI';
+import {
+  mergedSearch,


Please get rid of mergedSearch etc. You don't need to merge the search, simply perform each query against the named data source, providing the RetrieveAE in the keyed value.

wayfarer3130 · 2023-11-13T19:31:14Z

extensions/default/src/DicomWebDataSource/index.js

-            }) || {};
-
-          const results = await qidoSearch(qidoDicomWebClient, undefined, undefined, mappedParams);
+          const clients = dicomWebClientManager.getQidoClients();


Just separate out the clients into separate configs, and lookup the global config with the given name. That way you can have multiple top level query entry points, and multiple users of those. That is:
RetrieveAE => query locaiton 1 with study data for some specific purpose
RetrieveAE 2 - global study data with global study query
AE 2 points to AE 1 for AE1 specific data, but otherwise AE 1 points to AE 2 for everything imaging related.
There are lots of situations like this. A true merge AE is a merge of the study level query results, and should be it's own data source type that is JUST merge.

wayfarer3130 · 2023-11-13T19:33:19Z

extensions/default/src/DicomWebDataSource/utils/mergeUtils.js

+ * @param {Function} options.qidoSearch - The function to perform the QIDO search.
+ * @returns {Promise<Array>} - A promise that resolves to the merged search results.
+ */
+export const mergedSearch = async ({ clients, origParams, mapParams, qidoSearch }) => {


If you truly need merge search results, then you need to define how the merge happens, and that should be in a separate data source type, NOT in the DICOMweb type. That is, you might merge two DICOMweb data sources, and another one that is a different type entirely. Mixing it up in DICOMweb code is going to cause endless bugs.

It's not part of the plan to merge conflicting results. The assumption is that each server will contain a unique set of data. Real conciliation of data is much more complex and not part of the scope of the PR.

wayfarer3130 · 2023-11-13T19:34:28Z

...ions/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.tsx

+                        await dataSource.reject.series(
+                          ds.StudyInstanceUID,
+                          ds.SeriesInstanceUID,
+                          ds.instances[0].clientName


ds.instance.RetrieveAETitle
You should have a single instance that defines the series rather than expecting instance 0 to be what you are viewing.

igoroctaviano · 2023-12-08T11:39:17Z

Delivered by #3788

rodrigobasilio2022 added the IDC:priority Items that the Imaging Data Commons wants to help sponsor label Sep 6, 2023

rodrigobasilio2022 changed the title ~~[WIP] feat(retrieve) : Allow OHIF query and retrieve from mroe than one server~~ [WIP] feat(retrieve) : Allow OHIF query and retrieve from more than one server Sep 6, 2023

rodrigobasilio2022 force-pushed the feat/TwoServers branch from 82e871b to 83acbd5 Compare September 6, 2023 16:47

rodrigobasilio2022 changed the title ~~[WIP] feat(retrieve) : Allow OHIF query and retrieve from more than one server~~ [WIP] feat(retrieve) : Allow OHIF query and retrieve from multiple servers Sep 6, 2023

rodrigobasilio2022 changed the title ~~[WIP] feat(retrieve) : Allow OHIF query and retrieve from multiple servers~~ [WIP] feat(retrieve) : Allow OHIF retrieve from multiple servers Sep 6, 2023

rodrigobasilio2022 changed the title ~~[WIP] feat(retrieve) : Allow OHIF retrieve from multiple servers~~ feat(retrieve) : Allow OHIF retrieve from multiple servers Sep 6, 2023

rodrigobasilio2022 requested review from sedghi and igoroctaviano September 6, 2023 19:42

igoroctaviano requested changes Sep 8, 2023

View reviewed changes

extensions/cornerstone-dicom-rt/src/loadRTStruct.js Show resolved Hide resolved

extensions/default/src/DicomWebDataSource/retrieveStudyMetadata.js Outdated Show resolved Hide resolved

extensions/default/src/DicomWebDataSource/utils/clientManager.ts Outdated Show resolved Hide resolved

rodrigobasilio2022 added 11 commits September 13, 2023 17:29

Fix typo errors

63a1600

Initial commit clientManager

0c03bbc

retrieve async stores wado root correctly

adcf868

Add multiple server search in sync retrieve

57e216f

Undi study search in all servers

7623b76

Restoring default.js

8595f24

Adds documentation

b48bd50

Restoring qido.js

30e7de0

Check series duplicates

c85daf8

Fix number of instances in study list

4b84dda

Fix bulkdataURI and store dicom bugs

cd0db23

rodrigobasilio2022 force-pushed the feat/TwoServers branch from a24bd7a to cd0db23 Compare September 14, 2023 08:25

rodrigobasilio2022 added 2 commits September 14, 2023 09:24

Add secondGoogleserver parameter

ec5ddbe

Restore default.js

93e6011

rodrigobasilio2022 requested a review from igoroctaviano September 15, 2023 09:41

rodrigobasilio2022 self-assigned this Sep 15, 2023

igoroctaviano requested changes Sep 15, 2023

View reviewed changes

extensions/default/src/DicomWebDataSource/utils/clientManager.ts Outdated Show resolved Hide resolved

extensions/default/src/DicomWebDataSource/utils/clientManager.ts Outdated Show resolved Hide resolved

igoroctaviano unassigned rodrigobasilio2022 Nov 3, 2023

rodrigobasilio2022 added 3 commits November 7, 2023 08:17

Merge branch 'master' of https://github.com/rodrigobasilio2022/Viewers …

6d60577

…into feat/TwoServers

Refactor code as reviewer suggestions

3b96a10

Refactor as suggested by reviewer

eb88378

igoroctaviano and others added 10 commits November 9, 2023 18:37

Merge branch 'master' of github.com:ohif/Viewers into feat/TwoServers

3516d15

Extract functions

69eef25

Extract functions

2c293d3

Add documentation

c76d580

Update documentation

22ba149

Rename manager

44329cb

Merge branch 'master' into feat/TwoServers

83aa0e8

Update authorization header call

2f2efb9

Merge branch 'feat/TwoServers' of github.com:rodrigobasilio2022/Viewe…

1a2e836

…rs into feat/TwoServers

Merge branch 'master' of github.com:ohif/Viewers into feat/TwoServers

002aa1a

igoroctaviano requested a review from sedghi November 13, 2023 16:25

Merge branch 'master' into feat/TwoServers

6f4f2e6

wayfarer3130 reviewed Nov 13, 2023

View reviewed changes

igoroctaviano closed this Dec 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(retrieve) : Allow OHIF retrieve from multiple servers #3633

feat(retrieve) : Allow OHIF retrieve from multiple servers #3633

rodrigobasilio2022 commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

codecov bot commented Sep 6, 2023 •

edited

Loading

sedghi commented Sep 7, 2023

igoroctaviano left a comment

igoroctaviano commented Nov 3, 2023

rodrigobasilio2022 commented Nov 7, 2023

sedghi commented Nov 13, 2023

igoroctaviano commented Nov 13, 2023

sedghi commented Nov 13, 2023 •

edited

Loading

rodrigobasilio2022 commented Nov 13, 2023

wayfarer3130 commented Nov 13, 2023

wayfarer3130 Nov 13, 2023

wayfarer3130 Nov 13, 2023

wayfarer3130 Nov 13, 2023

wayfarer3130 Nov 13, 2023

igoroctaviano Nov 13, 2023

wayfarer3130 Nov 13, 2023

igoroctaviano commented Dec 8, 2023

feat(retrieve) : Allow OHIF retrieve from multiple servers #3633

feat(retrieve) : Allow OHIF retrieve from multiple servers #3633

Conversation

rodrigobasilio2022 commented Sep 6, 2023 • edited Loading

Context

Changes & Results

Testing

Checklist

PR

Code

Public Documentation Updates

Tested Environment

netlify bot commented Sep 6, 2023 • edited Loading

✅ Deploy Preview for ohif-platform-docs ready!

netlify bot commented Sep 6, 2023 • edited Loading

✅ Deploy Preview for ohif-dev ready!

codecov bot commented Sep 6, 2023 • edited Loading

Codecov Report

sedghi commented Sep 7, 2023

igoroctaviano left a comment

Choose a reason for hiding this comment

igoroctaviano commented Nov 3, 2023

rodrigobasilio2022 commented Nov 7, 2023

sedghi commented Nov 13, 2023

igoroctaviano commented Nov 13, 2023

sedghi commented Nov 13, 2023 • edited Loading

rodrigobasilio2022 commented Nov 13, 2023

wayfarer3130 commented Nov 13, 2023

wayfarer3130 Nov 13, 2023

Choose a reason for hiding this comment

wayfarer3130 Nov 13, 2023

Choose a reason for hiding this comment

wayfarer3130 Nov 13, 2023

Choose a reason for hiding this comment

wayfarer3130 Nov 13, 2023

Choose a reason for hiding this comment

igoroctaviano Nov 13, 2023

Choose a reason for hiding this comment

wayfarer3130 Nov 13, 2023

Choose a reason for hiding this comment

igoroctaviano commented Dec 8, 2023

rodrigobasilio2022 commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

netlify bot commented Sep 6, 2023 •

edited

Loading

codecov bot commented Sep 6, 2023 •

edited

Loading

sedghi commented Nov 13, 2023 •

edited

Loading