Skip to content

Commit

Permalink
Add support for DICOMweb range requests
Browse files Browse the repository at this point in the history
When a range request is made or a fuse mount is attempted, we compute the
size of the DICOM file. Currently, the only reliable way that we know to do
this is by streaming the whole file and counting the total bytes read.

After the file size is computed, it is saved in `file['size']` so it does not
need to be computed again. Then, the range requests can be successfully made,
complete with accurate `Content-Length` and `Content-Range` headers.

Signed-off-by: Patrick Avery <[email protected]>
  • Loading branch information
psavery committed Jan 26, 2024
1 parent 78fa459 commit 4760bda
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import cherrypy
import requests
from large_image_source_dicom.dicom_tags import dicom_key_to_tag
from large_image_source_dicom.dicomweb_utils import get_dicomweb_metadata
Expand Down Expand Up @@ -105,14 +106,39 @@ def deleteFile(self, file):
# We don't actually need to do anything special
pass

def setContentHeaders(self, file, offset, endByte, contentDisposition=None):
"""
Sets the Content-Length, Content-Disposition, Content-Type, and also
the Content-Range header if this is a partial download.
:param file: The file being downloaded.
:param offset: The start byte of the download.
:type offset: int
:param endByte: The end byte of the download (non-inclusive).
:type endByte: int or None
:param contentDisposition: Content-Disposition response header
disposition-type value, if None, Content-Disposition will
be set to 'attachment; filename=$filename'.
:type contentDisposition: str or None
"""
isRangeRequest = cherrypy.request.headers.get('Range')
setResponseHeader('Content-Type', file['mimeType'])
setContentDisposition(file['name'], contentDisposition or 'attachment')

if file.get('size') is not None:
# Only set Content-Length and range request headers if we have a file size
size = file['size']
if endByte is None or endByte > size:
endByte = size

Check warning on line 132 in sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py

View check run for this annotation

Codecov / codecov/patch

sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py#L132

Added line #L132 was not covered by tests

setResponseHeader('Content-Length', max(endByte - offset, 0))

if offset or endByte < size or isRangeRequest:
setResponseHeader('Content-Range', f'bytes {offset}-{endByte - 1}/{size}')

def downloadFile(self, file, offset=0, headers=True, endByte=None,
contentDisposition=None, extraParameters=None, **kwargs):

if offset != 0 or endByte is not None:
# FIXME: implement range requests
msg = 'Range requests are not yet implemented'
raise NotImplementedError(msg)

from dicomweb_client.web import _Transaction

dicom_uids = file['dicom_uids']
Expand All @@ -123,15 +149,8 @@ def downloadFile(self, file, offset=0, headers=True, endByte=None,
client = _create_dicomweb_client(self.assetstore_meta)

if headers:
setResponseHeader('Content-Type', file['mimeType'])
setContentDisposition(file['name'], contentDisposition or 'attachment')

# The filesystem assetstore calls the following function, which sets
# the above and also sets the range and content-length headers:
# `self.setContentHeaders(file, offset, endByte, contentDisposition)`
# However, we can't call that since we don't have a great way of
# determining the DICOM file size without downloading the whole thing.
# FIXME: call that function if we find a way to determine file size.
setResponseHeader('Accept-Ranges', 'bytes')
self.setContentHeaders(file, offset, endByte, contentDisposition)

# Create the URL
url = client._get_instances_url(
Expand All @@ -148,14 +167,39 @@ def downloadFile(self, file, offset=0, headers=True, endByte=None,
'type="application/dicom"',
f'transfer-syntax={transfer_syntax}',
]
headers = {
request_headers = {
'Accept': '; '.join(accept_parts),
}

def stream():
# Perform the request
response = client._http_get(url, headers=headers, stream=True)
yield from self._stream_retrieve_instance_response(response)
response = client._http_get(url, headers=request_headers, stream=True)

bytes_read = 0
for chunk in self._stream_retrieve_instance_response(response):
if bytes_read < offset:
# We haven't reached the start of the offset yet
bytes_needed = offset - bytes_read
if bytes_needed >= len(chunk):
# Skip over the whole chunk...
bytes_read += len(chunk)
continue

Check warning on line 186 in sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py

View check run for this annotation

Codecov / codecov/patch

sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py#L185-L186

Added lines #L185 - L186 were not covered by tests
else:
# Discard all bytes before the offset
chunk = chunk[bytes_needed:]
bytes_read += bytes_needed

if endByte is not None and bytes_read + len(chunk) >= endByte:
# We have reached the end... remove all bytes after endByte
chunk = chunk[:endByte - bytes_read]
if chunk:
yield chunk

bytes_read += len(chunk)
break

yield chunk
bytes_read += len(chunk)

return stream

Expand Down Expand Up @@ -374,6 +418,23 @@ def importData(self, parent, parentType, params, progress, user, **kwargs):
def auth_session(self):
return _create_auth_session(self.assetstore_meta)

def getFileSize(self, file):
# This function will compute the size of the DICOM file (a potentially
# expensive operation, since it may have to stream the whole file),
# and cache the result in file['size'].
# This function is called when the size is needed, such as the girder
# fuse mount code, and range requests.
if file.get('size') is not None:
# It has already been computed once. Return the cached size.
return file['size']

Check warning on line 429 in sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py

View check run for this annotation

Codecov / codecov/patch

sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py#L429

Added line #L429 was not covered by tests

size = 0
for chunk in self.downloadFile(file, headers=False)():
size += len(chunk)

# This should get cached in file['size'] in File().updateSize().
return size


def _create_auth_session(meta):
auth_type = meta.get('auth_type')
Expand Down
31 changes: 30 additions & 1 deletion sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('DICOMWeb assetstore', function () {
var destinationType;
var itemId;
var fileId;
var dicomFileContent;

// After importing, we will verify that this item exists
const verifyItemName = '1.3.6.1.4.1.5962.99.1.3205815762.381594633.1639588388306.2.0';
Expand Down Expand Up @@ -235,7 +236,35 @@ describe('DICOMWeb assetstore', function () {
});

// Should be larger than 500k bytes
return resp.status === 200 && resp.responseText.length > 500000;
const success = resp.status === 200 && resp.responseText.length > 500000;
if (!success) {
return false;
}

// Save the response text so we can compare with range requests...
dicomFileContent = resp.responseText;
return true;
}, 'Wait to download a single DICOM file');

// Verify that we can make a range request
waitsFor(function() {
const resp = girder.rest.restRequest({
url: 'file/' + fileId + '/download',
type: 'GET',
async: false,
headers: {
'Range': 'bytes=1000-1250'
},
});

return (
// 206 is a partial content response
resp.status === 206 &&
// Should be exactly 251 bytes
resp.responseText.length === 251 &&
// It should be equivalent to the slice of the file content
resp.responseText === dicomFileContent.slice(1000, 1251)
);
}, 'Wait for DICOM range request');
});
});

0 comments on commit 4760bda

Please sign in to comment.