rsna/s4s-fhir-broker

This web application is part of the imaging extension to the NIH sync for science program. It accepts RESTful calls on behalf of an EHR and brokers them to existing PACS.

s4s-fhir-broker's Issues

Endpoints are not externally accessible

Currently the FHIR broker returns ImagingStudy results like:

imaging_studies = requests.get(
  "https://broker.imaging.syncfor.science/baseDstu3/" +
  "ImagingStudy?patient=%s"%patient_id,
  headers={
    "Authorization": "Bearer %s"%access_token
}).json()

endpoints = [
  s['resource']['contained'][0]['address']
  for s in imaging_studies.get('entry')]

Which provides endpoints like:

['http://dcmrs-broker:4567/wado-rs/studies/1.3.6.1.4.1.14519.5.2.1.6279.6001.270617793']

This endpoint is using a reference-stack-internal URI, rather than a publicly accessible URL -- which means that a client can't actually dereference it to retrieve imaging study content. Instead, we should either:

1. Use a URL that is (somehow known to be) externally accessible
2. Provide a URL pointing to a fhir-broker-implemented proxy to ensure that it's acessible (e.g. "https://fhir-broker-url.example.org/Patient/123/$wado-rs/studies/example-study-uid)

Proxying queries to wado should use FHIR Operation syntax

Right now proxied queries look like [fhir base url]/studies/:id. Would be good to update the syntax so avoid potential conflict with FHIR resource names (like studies). My suggestion to make it look like a FHIR operation is:

[fhir base url]/$wado-rs?StudyInstanceUID=:id

Use prepared statements in SQL for PID get/put

This code for looking up FHIR IDs winds up directly executing statements that include non-sanitized inputs -- this can lead to SQL injection. For example:

s = "INSERT INTO pid_lookup VALUES ('" + pidIn + "', '" + pidOut + "')";

Can this be updated to use prepared statements?

ImagingStudy.endpoint.reference should point to a FHIR Endpoint resource

Currently ImagingStudy.endpoint.reference is populated with a string indicating a Wado endpoint URL. The FHIR specification expects this to be a reference to an Endpoint resource (not my favorite part about FHIR's design, but this is the spec). So instead of https://imaging-server/path/to/wado this should be something like:

Using Contained Resources

A resource like /ImagingStudy/123 could have a body like:

{
  "resourceType": "ImagingStudy",
  "id": "123",
  "contained": [{
    "id": "my-endpoint",
    "resourceType": "Endpoint",
    "address": "https://imaging-server/path/to/wado"
  }],
  "endpoint": {
    "reference": "#my-endpoint"
  }
}

Using an external Endpoint resource

A resource like /ImagingStudy/123 could have a body like:

{
  "resourceType": "ImagingStudy",
  "id": "123",
  "endpoint": {
    "reference": "Endpoint/wado"
  }
}

and then another resource exposed at /Endpoint/wado:

{
    "id": "my-endpoint",
    "resourceType": "Endpoint",
    "address": "https://imaging-server/path/to/wado"
}

Invalid volume in FHIR Broker Dockerfile

The following code:

s4s-fhir-broker/hapi-fhir-jpaserver-example/Dockerfile

Line 3 in 42872de

VOLUME data

Creates a volume which tries to be relative to the workdir (/var/lib/jetty) -- but this doesn't work as expected.

I think this needs to be an absolute path.

After adding an override, for example, docker-inspect shows two volumes in a container instead of one:

        "Mounts": [
            {
                "Type": "volume",
                "Name": "s4sstack_s4s-fhir-broker-vol",
                "Source": "/var/lib/docker/volumes/s4sstack_s4s-fhir-broker-vol/_data",
                "Destination": "/var/lib/jetty/data",
                "Driver": "local",
                "Mode": "rw",
                "RW": true,
                "Propagation": ""
            },
            {
                "Type": "volume",
                "Name": "b98929f885240332553c704a387983ef72b8b40c6d5dd369982334be648bbd6b",
                "Source": "/var/lib/docker/volumes/b98929f885240332553c704a387983ef72b8b40c6d5dd369982334be648bbd6b/_data",
                "Destination": "data",
                "Driver": "local",
                "Mode": "",
                "RW": true,
                "Propagation": ""
            }
        ],

It it possible to avoid 71 MB WAR file in repository?

It looks like s4s-fhir-broker/hapi-fhir-jpaserver-example/target/hapi-fhir-jpaserver-example.war is a 71MB .war file directly in the repository. Can we think about strategies for building this artifact when/where needed (e.g. as part of a docker build) rather than checking the result into version control? I'm concerned about the impact on developer workflow, especial when/if this file changes.