Fix/v3 dicomweb client prefixs #2767

kzgrzendek · 2022-04-02T13:30:24Z

Dicomweb requests fixes

Refactoring the usage of the dicomweb client to keep one client for all the dicomweb-related operations (still keeping the staticWado client appart)
All relevant details are mentioned here -> https://community.ohif.org/t/qido-wado-endpoints-handling-in-ohif-v3/24/8
Updated config file templates and docs
Also updated the base node image in the dockerfile, the one from the v3/stable was causing issues in the image build process
Slight changes in the DicomWeb Datasource config (needs to specify qido, wado and stow prefixes in the config file)
@swederik this is what we talk last month :) don't hesitate to tell me if you need me to modify anything

Merge branch 'v3-stable' into fix/v3-dicomweb-client-prefixs

… & STOW)

netlify · 2022-04-02T13:30:30Z

✅ Deploy Preview for ohif-platform-docs ready!

Name	Link
🔨 Latest commit	`8d48cf2`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-docs/deploys/64b41968fefbf00008ed5afb
😎 Deploy Preview	https://deploy-preview-2767--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 · 2022-04-02T13:30:31Z

✅ Deploy Preview for ohif-platform-viewer ready!

Name	Link
🔨 Latest commit	`47cef51`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-platform-viewer/deploys/644ced2f5d0ed30008ea1d78
😎 Deploy Preview	https://deploy-preview-2767--ohif-platform-viewer.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 settings.

codecov · 2022-04-02T13:33:44Z

Codecov Report

Merging #2767 (8d48cf2) into master (18156bb) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2767   +/-   ##
=======================================
  Coverage   42.75%   42.75%           
=======================================
  Files          82       82           
  Lines        1450     1450           
  Branches      338      338           
=======================================
  Hits          620      620           
  Misses        667      667           
  Partials      163      163

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 265c1fb...8d48cf2. Read the comment docs.

sedghi · 2022-11-10T18:23:32Z

@wayfarer3130 can you please review this after you come back from vacation?

@kzgrzendek why we are changing qidoRoot to qidoPrefixURL? (and others)

kzgrzendek · 2022-11-14T17:33:14Z

@wayfarer3130 can you please review this after you come back from vacation?

@kzgrzendek why we are changing qidoRoot to qidoPrefixURL? (and others)

Hi @sedghi! The goal is to adapt to the way the dicomweb-client we use is working -> https://github.com/dcmjs-org/dicomweb-client/blob/447dfe040b454037ed86d55a1ad35304dbb5041e/src/api.js#L62

It expects a base URL, then different prefixs for the dicomweb endpoint you want to consume. That way, we use the same dicom web client for QIDO, WADO and STOW.

I've detailed my actions in a series of (too) long posts in the OHIF Community forum -> https://community.ohif.org/t/qido-wado-endpoints-handling-in-ohif-v3/24/8

PS : I've updated the PR with the laste version of the ohif-v3 branch to resolve conflicts. I've also fixed the Dockerfile that wasn't building at all.

wayfarer3130 · 2022-11-15T12:53:54Z

For a single web service, the DICOMweb definition in part 18 specifies:
The Studies Resource enables a user agent to store, retrieve, update, and search an origin server for DICOM Studies, Series, and Instances, along with their /metadata, /rendered, and /thumbnail variants; as well as Frames and Bulkdata.

That actually means that if the studies query response returns a response, you are supposed to be able to fetch data etc from the same path on it. Now, you can simply say that they are three unrelated services, but the standard's compliant method of doing that is to have the study response include the Tag RetrieveURL included in the response. Note that this tag MAY be included for series level queries and be different for different series. It is also allowed to return the RetrieveAE field, and key off the AE name for the next retrieve level. You can also have some proprietary method to determine the next level, such as having fixed AE names (aka data source names), or the custom prefixes you suggested.

It would be very useful instead of having the logic to determine the paths be embedded in the dicomweb client, to have that determined by the data source provider, and to simply define multiple data sources in the case that they reference different URLs. That way, one could define a WADO URI retriever, a WADO RS one on several paths, and some sort of method to determine which data source is used for a given level/mapping. That is, unfortunately, harder to implement. I think we can probably get by with fewer changes by having some of the values be optional. Let me try to make some suggestions in the files themselves.

wayfarer3130 · 2022-11-15T12:56:28Z

extensions/default/src/DicomWebDataSource/index.js

- * @param {boolean} qidoSupportsIncludeField - Whether QIDO supports the "Include" option to request additional fields in response
- * @param {string} imageRengering - wadors | ? (unsure of where/how this is used)
- * @param {string} thumbnailRendering - wadors | ? (unsure of where/how this is used)
+ * @param {string} baseUrl - The root URL for DICOMWeb calls


Have the names matche the dicomweb client definition - that is, it should take url as the base url parameter, and ONLY add wadoURLPrefix etc if specified.

wayfarer3130 · 2022-11-15T12:57:55Z

extensions/default/src/DicomWebDataSource/index.js

 * @param {string|bool} singlepart - indicates of the retrieves can fetch singlepart.  Options are bulkdata, video, image or boolean true
 */
 function createDicomWebApi(dicomWebConfig, UserAuthenticationService) {
  const {
-    qidoRoot,


Just extract the ...options from the parameter, so that you can pass any additional options in as required.

wayfarer3130 · 2022-11-15T12:58:51Z

extensions/default/src/DicomWebDataSource/index.js

-    url: wadoRoot,
-    staticWado,
-    singlepart,
+  const dicomWebClientConfig = {


Just pass in options directly, or else use
const wadoConfig = {
...options,
<any custom over-rides}
}

wayfarer3130 · 2022-11-15T13:03:10Z

extensions/default/src/DicomWebDataSource/index.js

@@ -220,16 +207,16 @@ function createDicomWebApi(dicomWebConfig, UserAuthenticationService) {
          (hasAccept ? '' : (hasQuery ? '&' : '?') + `accept=${defaultType}`);
        if (BulkDataURI.indexOf('http') === 0) return acceptUri;
        if (BulkDataURI.indexOf('/') === 0) {
-          return wadoRoot + acceptUri;
+          return `${baseUrl}/${wadoURLPrefix}/${acceptUri}`;


Create a function on the data access object which gets the URL for a given query type, defaulting to the base URL. That way there is just a single query generator.

You could put the getter function on the config object, so that it can be used for doing things like generating the wadors URI's. Something like
if( !config.getUrl ) {
config.getUrl = function(typeId, ...additions)
to get the URL. It should handle query parameters defined in the base URL or in the suffixes as well.... typeId would be 'wado' or 'qido' etc.

wayfarer3130

This should be more consistent with the dicomweb client if that is the direction we want to take - that is, use url and then only define the prefixes as necessary. Encapsulate the logic to do all that.

wayfarer3130 · 2022-11-15T13:07:11Z

extensions/default/src/DicomWebDataSource/index.js

@@ -220,16 +207,16 @@ function createDicomWebApi(dicomWebConfig, UserAuthenticationService) {
          (hasAccept ? '' : (hasQuery ? '&' : '?') + `accept=${defaultType}`);
        if (BulkDataURI.indexOf('http') === 0) return acceptUri;
        if (BulkDataURI.indexOf('/') === 0) {
-          return wadoRoot + acceptUri;
+          return `${baseUrl}/${wadoURLPrefix}/${acceptUri}`;


You could put the getter function on the config object, so that it can be used for doing things like generating the wadors URI's. Something like
if( !config.getUrl ) {
config.getUrl = function(typeId, ...additions)
to get the URL. It should handle query parameters defined in the base URL or in the suffixes as well.... typeId would be 'wado' or 'qido' etc.

wayfarer3130 · 2022-11-15T13:11:33Z

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

@@ -13,7 +13,7 @@ function buildInstanceWadoUrl(config, instance) {

  const paramString = params.join('&');

-  return `${config.wadoUriRoot}?${paramString}`;
+  return `${config.baseUrl}/${config.wadoURLPrefix}?${paramString}`;


Use the function added to the config to get the query parameters/setup, eg make it:
config.getUrl('wado', ?{paramString})

wayfarer3130 · 2022-11-15T13:12:53Z

platform/core/src/utils/addServer.test.js

@@ -6,8 +6,9 @@ describe('addServers', () => {
      {
        name: 'DCM4CHEE',
        wadoUriRoot: 'https://server.dcmjs.org/dcm4chee-arc/aets/DCM4CHEE/wado',
-        qidoRoot: 'https://server.dcmjs.org/dcm4chee-arc/aets/DCM4CHEE/rs',
-        wadoRoot: 'https://server.dcmjs.org/dcm4chee-arc/aets/DCM4CHEE/rs',
+        qidoUrlPrefix: 'https://server.dcmjs.org/dcm4chee-arc/aets/DCM4CHEE/rs',


Should all be replaced with just uri. Then, if necessary, specific versions can include the prefix definitions as needed.

sedghi · 2023-06-19T13:30:08Z

@kzgrzendek Thanks for this PR, any one free you have to work on this?

kzgrzendek · 2023-06-27T13:01:25Z

@kzgrzendek Thanks for this PR, any one free you have to work on this?

Hello @sedghi, not at the moment. I can have a look at it on my free time, but atm I'm very busy on other topics, and unfortunately not allowed to do it on my working hours for legal reasons. If you have people available for doing the requested changes, I'd be glad to assist! If not, I'll try to have a look at it in the following days/week but can't give any estimations unfortunately

…t-prefixs

netlify · 2023-07-16T16:19:46Z

✅ Deploy Preview for ohif-dev ready!

Name	Link
🔨 Latest commit	`8d48cf2`
🔍 Latest deploy log	https://app.netlify.com/sites/ohif-dev/deploys/64b41968d4b17b000876e2e2
😎 Deploy Preview	https://deploy-preview-2767--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.

Kévin ZGRZENDEK added 5 commits March 12, 2022 16:21

Updated dicomweb-client dependency to latest stable

860925e

Updated with latest version of v3-stable branch

0937250

Merge branch 'v3-stable' into fix/v3-dicomweb-client-prefixs

Added a unique dicomweb client for all the backends calls (QIDO, WADO…

793e638

… & STOW)

Updated doc & UT

aeb59f0

Fixed issue with build on docker

6922f2b

kzgrzendek added 2 commits November 14, 2022 17:26

Updated with latest v3 cersion and fixed Docker build

742ae03

Fixed reference to old params in the image IDs used for dicomweb calls

30feff0

wayfarer3130 reviewed Nov 15, 2022

View reviewed changes

wayfarer3130 requested changes Nov 15, 2022

View reviewed changes

Updated with last version of v3-stable and fixed adapted fix

47cef51

sedghi changed the base branch from v3-stable to master June 19, 2023 13:29

kzgrzendek added 2 commits June 27, 2023 15:23

Merge remote-tracking branch 'ohif/master' into fix/v3-dicomweb-clien…

3e97766

…t-prefixs

Adapted fix for headers resolution

f6a9937

Updated with latest master version and fixed conflicts

8d48cf2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/v3 dicomweb client prefixs #2767

Fix/v3 dicomweb client prefixs #2767

kzgrzendek commented Apr 2, 2022

netlify bot commented Apr 2, 2022 •

edited

Loading

netlify bot commented Apr 2, 2022 •

edited

Loading

codecov bot commented Apr 2, 2022 •

edited

Loading

sedghi commented Nov 10, 2022

kzgrzendek commented Nov 14, 2022 •

edited

Loading

wayfarer3130 commented Nov 15, 2022

wayfarer3130 Nov 15, 2022

wayfarer3130 Nov 15, 2022

wayfarer3130 Nov 15, 2022

wayfarer3130 Nov 15, 2022

wayfarer3130 Nov 15, 2022

wayfarer3130 left a comment

wayfarer3130 Nov 15, 2022

wayfarer3130 Nov 15, 2022

wayfarer3130 Nov 15, 2022

sedghi commented Jun 19, 2023

kzgrzendek commented Jun 27, 2023

netlify bot commented Jul 16, 2023 •

edited

Loading

Fix/v3 dicomweb client prefixs #2767

Are you sure you want to change the base?

Fix/v3 dicomweb client prefixs #2767

Conversation

kzgrzendek commented Apr 2, 2022

Dicomweb requests fixes

netlify bot commented Apr 2, 2022 • edited Loading

✅ Deploy Preview for ohif-platform-docs ready!

netlify bot commented Apr 2, 2022 • edited Loading

✅ Deploy Preview for ohif-platform-viewer ready!

codecov bot commented Apr 2, 2022 • edited Loading

Codecov Report

sedghi commented Nov 10, 2022

kzgrzendek commented Nov 14, 2022 • edited Loading

wayfarer3130 commented Nov 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wayfarer3130 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sedghi commented Jun 19, 2023

kzgrzendek commented Jun 27, 2023

netlify bot commented Jul 16, 2023 • edited Loading

✅ Deploy Preview for ohif-dev ready!

netlify bot commented Apr 2, 2022 •

edited

Loading

netlify bot commented Apr 2, 2022 •

edited

Loading

codecov bot commented Apr 2, 2022 •

edited

Loading

kzgrzendek commented Nov 14, 2022 •

edited

Loading

netlify bot commented Jul 16, 2023 •

edited

Loading