dcm4che / dcm4chee-arc-light

DICOM Archive J2EE application
439 stars 242 forks source link

Improved URLs for Invoke Image Display #2540

Closed nroduit closed 4 years ago

nroduit commented 4 years ago

Here is a suggestion to make the URL to open viewers more flexible.

Currently the mechanism constrains the use of specific parameters (according to the IID profile) to inject the identifiers of patient or study.

Here are modifications to the code to only inject values without constraining the format of the URL.

openViewer(model, mode){
    try {
        let token;
        let target;
        let url;
        let configuredUrlString = mode === "study" ? this.studyWebService.selectedWebService['IID_STUDY_URL'] : this.studyWebService.selectedWebService['IID_PATIENT_URL'];
        let stUID = model["0020000D"] != null ? model["0020000D"].Value[0] : "";
        let ptID = this.service.getPatientId(model);
        let rsService = this.studyWebService.selectedWebService['dcmWebServicePath'];
        let substitutions;
        let replaceDoubleBraces = (url, result) => {
            return url.replace(/{{(.+?)}}/g, (_, g1) => result[g1] == null ? g1 : result[g1])
        }
        this.service.getTokenService(this.studyWebService).subscribe((response) => {
            if (!this.appService.global.notSecure) {
                token = response.token;
            }
            target = configuredUrlString.search("target.self") ? configuredUrlString.search("target.blank") == -1  ? "" : "_blank" : "_self";
            substitutions = {
                "rs.service.path" : rsService,
                "rs.service.url" : `${window.location.origin}${rsService}`,
                "patient.id": ptID,
                "study.uid": stUID,
                "access.token": token,
                "target.self": "",
                "target.blank": ""
            };
            url = replaceDoubleBraces(configuredUrlString, substitutions).trim();
            console.log("Prepared URL: ", url);
            console.groupEnd();
            if (target) {
                window.open(encodeURI(url), target);
            } else {
                window.open(encodeURI(url));
            }
        });
    }catch(e){
        j4care.log("Something went wrong while opening the Viewer",e);
        this.appService.showError($localize `:@@study_error_on_opening_viewer:Something went wrong while opening the Viewer open the inspect to see more details`);
    }
};

The variables to be injected are enclosed in 2 brackets.

Note: {{target.blank}} and {{target.self}} are removed from the URL and pass as a parameter in window.open(url, target)

Benefits of this solution

Test the changes

  1. Add in docker-compose.env:
    IID_PATIENT_URL=weasis://$dicom:rs --url "{{rs.service.url}}" -r "patientID={{patient.id}}" --query-ext "&includedefaults=false" -H "Authorization: Bearer {{access.token}}"
    IID_STUDY_URL=weasis://$dicom:rs --url "{{rs.service.url}}" -r "studyUID={{study.uid}}" --query-ext "&includedefaults=false" -H "Authorization: Bearer {{access.token}}"
  2. Install Weasis and click on the view button in dcm4chee web portal.

Configuration with Invoke Image Display profile

It is still possible to use the IID URL with weasis-pacs-connector (need to be installed with dcm4chee)

IID_PATIENT_URL=../../weasis-pacs-connector/IHEInvokeImageDisplay?patientID={{patient.id}}&access_token={{access.token}}&cdb{{target.self}}
IID_STUDY_URL=../../weasis-pacs-connector/IHEInvokeImageDisplay?studyUID={{study.uid}}&access_token={{access.token}}&cdb{{target.self}}
shral commented 4 years ago

Hallo @nroduit thank you for your contribution! Injecting only the values without constraining the format of the URL is a good idea, I will discuss that with my colleagues and we will see to what extent we can implement that the way how you suggested.

gunterze commented 4 years ago

Prefer to configure the target parameter by additional properties:

and use the query parameter names defined in IHE RAD Supplement IID as variable names:

Don't see the need for {{rs.service.path}} or {{rs.service.url}}. IID_PATIENT_URL and IID_STUDY_URL properties have to be configured individually for each Web Application, so you can directly specify the Path or URL of the related DICOMweb service in the property value.

shral commented 4 years ago

Prefer to configure the target parameter by additional properties:

  • IID_PATIENT_URL_TARGET = _blank|_self
  • IID_STUDY_URL_TARGET = _blank|_self

Using separate parameters for the target I agree is much cleaner but maybe we don't even need two separate parameters for the target, I mean why the user would want to open the viewer in the patient level differently from the study level?!

What I don't understand @gunterze why you thing one would need to pass a patient birthdate, patient name and current date-time? If we start like that shouldn't we then support all filters what we have for patient and study?

vrindanayak commented 4 years ago

What I don't understand @gunterze why you thing one would need to pass a patient birthdate, patient name and current date-time? If we start like that shouldn't we then support all filters what we have for patient and study?

This requirement (optional / conditional for certain fields) is as per RAD-106 Invoke Image Display profile - see pages 21, 22

gunterze commented 4 years ago

The target system may have stored the same Patient with another Patient ID, then Patient Name and Birthdate

Assists in the identification of the subject of the studies being requested, in case the patientID is unrecognized.

RAD-106 Invoke Image Display profile

... which of course is not the case, if the invoked Image DIsplay query/retrieve from this archive.

{{currentDateTime-P[{days}D][T[{hours}H][{min}M]}} may be passed as value of

Parameter Name REQ Description Notes
lowerDateTime O Used to constrain the earliest study date/time. Shall be encoded in the XML primitive dateTime format.
upperDateTime O Used to constrain the latest study date/time. Shall be encoded in the XML primitive dateTime format.

(Makes only sense for IID_PATIENT_URL)

nroduit commented 4 years ago

I fully agree with all the remarks made in relation to the parameters.

My request to have the DICOMweb service url as a parameter goes beyond the IID profile which does not deal with security and implementation aspects. In my opinion, it makes sense to deliver a token with an access to this data (the URL of the DICOMweb service).

Weasis fully implements in the client the IID request from the DICOMweb service. The only server configuration is: IID_PATIENT_URL=weasis://$dicom:rs --url "{{rs.service.url}}" -r "IID request" -H "Authorization: Bearer {{access.token}}"

nroduit commented 4 years ago

Another reason to use the URL of the DICOMweb service dynamically instead of configuring it in a property in a static way is to guarantee accessibility when there are several hostnames configured (e.g. one for the Intranet and one for the Internet).

gunterze commented 4 years ago

But then there is the question, how the archive UI determines the value for {{rs.service.url}, because the Study list now also supports to query external archives - either directly by QIDO or via a QIDO to C-FIND proxy. So the WADO-RS URL does not longer need to match the QIDO URL used by the UI to populate the list. Anyway, just always replace it with the QIDO (Base) URL used by the UI is feasible. So I would rather use {{qidoBaseURL}} and {{qidoBasePath}} as variable names.

nroduit commented 4 years ago

The implementation in Weasis makes quido queries until it gets the url to download an instance. By default it tries to retrieve Tag.RetrieveURL and if it doesn't exist (many PACS implementations don't return Tag.RetrieveURL), the instance URL is built manually (baseurl/.../instances/sopuid). Will Tag.RetrieveURL returned by dcm4chee allow to download the image even with C-FIND proxy?

If so then there should be no problems with {{qidoBaseURL}}.

nroduit commented 4 years ago

Thank you for considering this request.

It is possible to add {{qidoBaseURL}} in substitutions as described above ?

gunterze commented 4 years ago

Attention: The implementation now applies a common

and not individually

  • IID_PATIENT_URL_TARGET = _blank|_self
  • IID_STUDY_URL_TARGET = _blank|_self

as suggested above.

nroduit commented 4 years ago

Yes it makes sens to have only one parameter. I confirm that it works as expected on Firefox.