Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion dcm4chee-docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
version: "3"
services:
ldap:
image: dcm4che/slapd-dcm4chee:2.4.44-13.3
Expand Down
34 changes: 17 additions & 17 deletions src/api.js
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some fixes for the retrieveInstanceFrames, needed since I added an actual integration test for it, and that wasn't running until I added the fixes.

Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { multipartEncode, multipartDecode } from './message.js';


import { multipartEncode, multipartDecode, addHeaders } from './message.js';

function isObject(obj) {
return typeof obj === 'object' && obj !== null;
Expand Down Expand Up @@ -225,7 +223,7 @@ class DICOMwebClient {
let requestInstance = request.instance ? request.instance : new XMLHttpRequest();

requestInstance.open(method, url, true);
if ('responseType' in request) {
if (request.responseType) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excludes null/undefined which was causing issues.

requestInstance.responseType = request.responseType;
}

Expand Down Expand Up @@ -256,12 +254,16 @@ class DICOMwebClient {
requestInstance.onreadystatechange = () => {
if (requestInstance.readyState === 4) {
if (requestInstance.status === 200) {
const contentType = requestInstance.getResponseHeader('Content-Type');
const contentType = requestInstance.getResponseHeader(
'Content-Type',
);
const headers = requestInstance.getAllResponseHeaders();
// Automatically distinguishes between multipart and singlepart in an array buffer, and
// converts them into a consistent type.
if (contentType && contentType.indexOf('multipart') !== -1) {
resolve(multipartDecode(requestInstance.response));
} else if (requestInstance.responseType === 'arraybuffer') {
addHeaders(requestInstance.response, headers);
resolve([requestInstance.response]);
} else {
resolve(requestInstance.response);
Expand Down Expand Up @@ -295,10 +297,8 @@ class DICOMwebClient {
};

// Event triggered while download progresses
if ('progressCallback' in request) {
if (typeof request.progressCallback === 'function') {
requestInstance.onprogress = request.progressCallback;
}
if (typeof request.progressCallback === 'function') {
requestInstance.onprogress = request.progressCallback;
}

if (requestHooks && areValidRequestHooks(requestHooks)) {
Expand All @@ -311,13 +311,11 @@ class DICOMwebClient {
}

// Add withCredentials to request if needed
if ('withCredentials' in request) {
if (request.withCredentials) {
requestInstance.withCredentials = true;
}
if (request.withCredentials) {
requestInstance.withCredentials = true;
}

if ('data' in request) {
if (request.data) {
requestInstance.send(request.data);
} else {
requestInstance.send();
Expand Down Expand Up @@ -594,6 +592,7 @@ class DICOMwebClient {
'image/gif',
'image/png',
'image/jp2',
'image/*',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image/* is a valid type, and I was trying to get the generic retrieve working, but it doesn't seem to be allowed for dcm4chee

];
} else {
supportedMediaTypes = {
Expand All @@ -608,6 +607,7 @@ class DICOMwebClient {
'1.2.840.10008.1.2.4.91': ['image/jp2'],
'1.2.840.10008.1.2.4.92': ['image/jpx'],
'1.2.840.10008.1.2.4.93': ['image/jpx'],
'*': ['image/*'],
};

if (byteRange) {
Expand Down Expand Up @@ -961,7 +961,7 @@ class DICOMwebClient {
});

if( !fieldValueParts.length ) {
throw new Error(`No acceptable media types found among ${JSON.stringify(mediaTypes)}`);
throw new Error(`No acceptable media types found among ${JSON.stringify(mediaTypes)} testing against ${JSON.stringify(acceptableMediaTypes)}`);
}

return fieldValueParts.join(', ');
Expand Down Expand Up @@ -1227,7 +1227,7 @@ class DICOMwebClient {
debugLog(`retrieve metadata of instance ${options.sopInstanceUID}`);
const url = `${this.wadoURL}/studies/${options.studyInstanceUID}/series/${options.seriesInstanceUID}/instances/${options.sopInstanceUID}/metadata`;

const request = getRequestOptions(options.request)
const request = getRequestOptions(options.request);
return this._httpGetApplicationJson(url, {}, request);
}

Expand Down Expand Up @@ -1276,6 +1276,7 @@ class DICOMwebClient {
const { mediaTypes } = options;

const request = getRequestOptions(options.request)
request.responseType = 'arraybuffer';

if (!mediaTypes) {
return this._httpGetMultipartApplicationOctetStream(
Expand Down Expand Up @@ -1317,7 +1318,6 @@ class DICOMwebClient {
supportedMediaTypes,
),
};
request.responseType = 'arraybuffer';
return this._httpGet(url, headers, request);
}

Expand Down
69 changes: 65 additions & 4 deletions src/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function identifyBoundary(header) {
const parts = header.split('\r\n');

for (let i = 0; i < parts.length; i++) {
if (parts[i].substr(0, 2) === '--') {
if (parts[i].substring(0, 2) === '--') {
return parts[i];
}
}
Expand Down Expand Up @@ -179,16 +179,72 @@ function multipartEncode(
};
}

/**
* Splits the header string into parts and extracts the simple contentType
* and transferSyntaxUID, assigning them, plus the headers map into the destination object.
*
* @param {*} destination
* @param {string} headerString
*/
function addHeaders(destination, headerString) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows for retrieving the actual transfer syntax used in the image encoding. This is allowed by Part 18, but unfortunately not required there.

if (!headerString) {
return;
}
const headerLines = headerString.split('\r\n').filter(Boolean);
const headers = new Map();
let transferSyntaxUID = null,
contentType = null;

for (const line of headerLines) {
const colon = line.indexOf(':');
if (colon === -1) {
continue;
}
const name = line.substring(0, colon).toLowerCase();
const value = line.substring(colon + 1).trim();
if (headers.has(name)) {
headers.get(name).push(value);
} else {
headers.set(name, [value]);
}
if (name === 'content-type') {
const endSimpleType = value.indexOf(';');
contentType = value.substring(
0,
endSimpleType === -1 ? value.length : endSimpleType,
);
const transferSyntaxStart = value.indexOf('transfer-syntax=');
if (transferSyntaxStart !== -1) {
const endTsuid = value.indexOf(';', transferSyntaxStart);
transferSyntaxUID = value.substring(
transferSyntaxStart + 16,
endTsuid === -1 ? value.length : endTsuid,
);
}
}
}

Object.defineProperty(destination, 'headers', { value: headers });
Object.defineProperty(destination, 'contentType', { value: contentType });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having at least the contentType allows one to figure out what the encoding is close enough to allow testing for which decoder to use.

Object.defineProperty(destination, 'transferSyntaxUID', {
value: transferSyntaxUID,
});
}

/**
* Decode a Multipart encoded ArrayBuffer and return the components as an Array.
*
* @param {ArrayBuffer} response Data encoded as a 'multipart/related' message
* @returns {Array} The content
* @returns {Uint8Array[]} The content as an array of Uint8Array
* Each item shall have a contentType value, and a transferSyntaxUID if available,
* as well as the headers Map. See parseHeaders for output.
*
*/
function multipartDecode(response) {
// Use the raw data if it is provided in an appropriate format
const message = ArrayBuffer.isView(response) ? response : new Uint8Array(response);

const message = ArrayBuffer.isView(response)
? response
: new Uint8Array(response);
/* Set a maximum length to search for the header boundaries, otherwise
findToken can run for a long time
*/
Expand All @@ -211,6 +267,8 @@ function multipartDecode(response) {
const boundaryLength = boundary.length;
const components = [];

const headers = header.substring(boundary.length + 2);

let offset = boundaryLength;

// Loop until we cannot find any more boundaries
Expand Down Expand Up @@ -240,6 +298,8 @@ function multipartDecode(response) {
// Extract data from response message, excluding "\r\n"
const spacingLength = 2;
const data = response.slice(offset, boundaryIndex - spacingLength);
// TODO - extract header data on a per frame basis.
addHeaders(data, headers);

// Add the data to the array of results
components.push(data);
Expand All @@ -261,4 +321,5 @@ export {
multipartEncode,
multipartDecode,
guid,
addHeaders,
};
15 changes: 11 additions & 4 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ let dwc = new DICOMwebClient.api.DICOMwebClient({
url: 'http://localhost:8008/dcm4chee-arc/aets/DCM4CHEE/rs',
retrieveRendered: false,
});

describe('dicomweb.api.DICOMwebClient', function() {
//
// Note: you can add the following for debugging tests locally
Expand Down Expand Up @@ -124,10 +125,17 @@ describe('dicomweb.api.DICOMwebClient', function() {
sopInstanceUID:
'1.3.6.1.4.1.14519.5.2.1.2744.7002.325971588264730726076978589153',
frameNumbers: '1',
// The next line should work, but the server side is broken
// mediaTypes: [ {mediaType: 'image/*' }],
};

const frames = dwc.retrieveInstance(options);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't even use the await, so it was essentially an empty test, without checking if the response was valid.

});
const frames = await dwc.retrieveInstanceFrames(options);
expect(frames instanceof Array).toBe(true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the test at least checks that the response has an image frame, and for the given server, tests for uncompressed.

expect(frames.length).toBe(1);
expect(frames[0].contentType).toBe("application/octet-stream");
// The next line is the correct value for servers supporting image/*
//expect(frames[0].transferSyntaxUID).toBe('1.2.3');
}, 15000);

it('should retrieve a single instance', async function() {
// from sample.dcm
Expand Down Expand Up @@ -173,10 +181,9 @@ describe('dicomweb.api.DICOMwebClient', function() {
const options = {
studyInstanceUID: '999.999.3859744',
seriesInstanceUID: '999.999.94827453',
sopInstanceUID: '999.999.133.1996.1.1800.1.6.25',
};

const metadata = await dwc.retrieveInstanceMetadata(options);
const metadata = await dwc.retrieveSeriesMetadata(options);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since instance metadata is tested elsewhere, this tests just makes sure series metadata also works.


// TODO: Check why metadata is an array of objects, not just an object
const bulkDataOptions = {
Expand Down
2 changes: 1 addition & 1 deletion test_ci.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Clear any previous data from the last test run
rm -rf /tmp/dcm4chee-arc/db
rm -rf ./tmp/dcm4chee-arc/db

# now start dcm4chee archive and wait for it to startup
echo 'Starting dcm4chee Docker container'
Expand Down