Merge pull request #95 from erikogabrielsson/bugfix/dicom-rle-mediatype

Bugfix/dicom rle mediatype

Author: CPBridge

data/retrieve_instance_pixeldata.rle

@@ -14,6 +14,7 @@
     Dict,
     Iterator,
     List,
+    Mapping,
     Optional,
     Set,
     Sequence,
@@ -865,7 +866,9 @@ def _build_accept_header_field_value(
     def _build_multipart_accept_header_field_value(
         cls,
         media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None],
-        supported_media_types: Union[Dict[str, str], Set[str]]
+        supported_media_types: Union[
+            Mapping[str, Union[str, Tuple[str, ...]]], Set[str]
+        ]
     ) -> str:
         """Build an accept header field value for a multipart request message.
 
@@ -874,7 +877,9 @@ def _build_multipart_accept_header_field_value(
         media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None]
             Acceptable media types and optionally the UIDs of the corresponding
             transfer syntaxes
-        supported_media_types: Union[Dict[str, str], Set[str]]
+        supported_media_types: Union[
+            Mapping[str, Union[str, Tuple[str, ...]]], Set[str]
+        ]
             Set of supported media types or mapping of transfer syntaxes
             to their corresponding media types
 
@@ -902,7 +907,13 @@ def _build_multipart_accept_header_field_value(
             cls._assert_media_type_is_valid(media_type)
             field_value = f'multipart/related; type="{media_type}"'
             if isinstance(supported_media_types, dict):
-                if media_type not in supported_media_types.values():
+                media_type_in_supported_media_types = any(
+                    media_type == supported_media_types
+                    if isinstance(supported_media_types, str)
+                    else media_type in supported_media_types
+                    for supported_media_types in supported_media_types.values()
+                )
+                if not media_type_in_supported_media_types:
                     if not (media_type.endswith('/*') or
                             media_type.endswith('/')):
                         raise ValueError(
@@ -916,14 +927,23 @@ def _build_multipart_accept_header_field_value(
                                 f'Transfer syntax "{transfer_syntax_uid}" '
                                 'is not supported for requested resource.'
                             )
-                        expected_media_type = supported_media_types[
+                        expected_media_types = supported_media_types[
                             transfer_syntax_uid
                         ]
-                        if expected_media_type != media_type:
-                            have_same_type = (
-                                cls._parse_media_type(media_type)[0] ==
-                                cls._parse_media_type(expected_media_type)[0]
+                        if not isinstance(expected_media_types, tuple):
+                            expected_media_types = (expected_media_types, )
+                        if media_type not in expected_media_types:
+                            have_same_type = next(
+                                (
+                                    cls._same_media_type(
+                                        media_type, expected_media_type
+                                    )
+                                    for expected_media_type
+                                    in expected_media_types
+                                ),
+                                False
                             )
+
                             if (have_same_type and
                                     (media_type.endswith('/*') or
                                         media_type.endswith('/'))):
@@ -1065,13 +1085,13 @@ def _http_get_multipart(
         default_media_type = '*/*'
         supported_media_types = {
             '1.2.840.10008.1.2.1': 'application/octet-stream',
-            '1.2.840.10008.1.2.5': 'image/x-dicom-rle',
+            '1.2.840.10008.1.2.5': ('image/dicom-rle', 'image/x-dicom-rle'),
             '1.2.840.10008.1.2.4.50': 'image/jpeg',
             '1.2.840.10008.1.2.4.51': 'image/jpeg',
             '1.2.840.10008.1.2.4.57': 'image/jpeg',
             '1.2.840.10008.1.2.4.70': 'image/jpeg',
-            '1.2.840.10008.1.2.4.80': 'image/jls',
-            '1.2.840.10008.1.2.4.81': 'image/jls',
+            '1.2.840.10008.1.2.4.80': ('image/jls', 'image/x-jls'),
+            '1.2.840.10008.1.2.4.81': ('image/jls', 'image/x-jls'),
             '1.2.840.10008.1.2.4.90': 'image/jp2',
             '1.2.840.10008.1.2.4.91': 'image/jp2',
             '1.2.840.10008.1.2.4.92': 'image/jpx',
@@ -1189,13 +1209,13 @@ def _http_get_multipart_image(
         """  # noqa: E501
         headers = {}
         supported_media_types = {
-            '1.2.840.10008.1.2.5': 'image/x-dicom-rle',
+            '1.2.840.10008.1.2.5': ('image/dicom-rle', 'image/x-dicom-rle'),
             '1.2.840.10008.1.2.4.50': 'image/jpeg',
             '1.2.840.10008.1.2.4.51': 'image/jpeg',
             '1.2.840.10008.1.2.4.57': 'image/jpeg',
             '1.2.840.10008.1.2.4.70': 'image/jpeg',
-            '1.2.840.10008.1.2.4.80': 'image/jls',
-            '1.2.840.10008.1.2.4.81': 'image/jls',
+            '1.2.840.10008.1.2.4.80': ('image/jls', 'image/x-jls'),
+            '1.2.840.10008.1.2.4.81': ('image/jls', 'image/x-jls'),
             '1.2.840.10008.1.2.4.90': 'image/jp2',
             '1.2.840.10008.1.2.4.91': 'image/jp2',
             '1.2.840.10008.1.2.4.92': 'image/jpx',
@@ -1661,6 +1681,27 @@ def search_for_studies(
             get_remaining=get_remaining
         )
 
+    @classmethod
+    def _same_media_type(cls, first: str, second: str) -> bool:
+        """Check if two media types have the same type.
+
+        Parameters
+        ----------
+        first: str
+            First media type
+        second: str
+            Second media type
+
+        Returns
+        -------
+        bool
+            Whether media types have the same type
+
+        """
+        return (
+            cls._parse_media_type(first)[0] == cls._parse_media_type(second)[0]
+        )
+
     @classmethod
     def _parse_media_type(cls, media_type: str) -> Tuple[str, str]:
         """Parse media type and extract its type and subtype.

src/dicomweb_client/web.py

@@ -880,22 +880,31 @@ def test_retrieve_instance_frames_jp2(httpserver, client, cache_dir):
     assert request.accept_mimetypes[0][0][:35] == headers['content-type'][:35]
 
 
-def test_retrieve_instance_frames_jls(httpserver, client, cache_dir):
+@pytest.mark.parametrize(
+    "media_type", ["image/dicom-rle", "image/x-dicom-rle"]
+)
+def test_retrieve_instance_frames_jls(
+    httpserver,
+    client,
+    cache_dir,
+    media_type
+):
     cache_filename = str(cache_dir.joinpath('retrieve_instance_pixeldata.jls'))
     with open(cache_filename, 'rb') as f:
         content = f.read()
-    headers = {
-        'content-type': 'multipart/related; type="image/jls"',
-    }
+    headers = {'content-type': f'multipart/related; type="{media_type}"',}
     httpserver.serve_content(content=content, code=200, headers=headers)
     study_instance_uid = '1.2.3'
     series_instance_uid = '1.2.4'
     sop_instance_uid = '1.2.5'
     frame_numbers = [114]
     frame_list = ','.join([str(n) for n in frame_numbers])
     result = client.retrieve_instance_frames(
-        study_instance_uid, series_instance_uid, sop_instance_uid,
-        frame_numbers, media_types=('image/jls', )
+        study_instance_uid,
+        series_instance_uid,
+        sop_instance_uid,
+        frame_numbers,
+        media_types=(media_type, )
     )
     assert list(result) == [content]
     request = httpserver.requests[0]
@@ -908,6 +917,43 @@ def test_retrieve_instance_frames_jls(httpserver, client, cache_dir):
     assert request.path == expected_path
     assert request.accept_mimetypes[0][0][:35] == headers['content-type'][:35]
 
+@pytest.mark.parametrize(
+    "media_type", ["image/dicom-rle", "image/x-dicom-rle"]
+)
+def test_retrieve_instance_frames_rle(
+    httpserver,
+    client,
+    cache_dir,
+    media_type
+):
+    cache_filename = str(cache_dir.joinpath('retrieve_instance_pixeldata.rle'))
+    with open(cache_filename, 'rb') as f:
+        content = f.read()
+    headers = {'content-type': f'multipart/related; type="{media_type}"',}
+    httpserver.serve_content(content=content, code=200, headers=headers)
+    study_instance_uid = '1.2.3'
+    series_instance_uid = '1.2.4'
+    sop_instance_uid = '1.2.5'
+    frame_numbers = [114]
+    frame_list = ','.join([str(n) for n in frame_numbers])
+    result = client.retrieve_instance_frames(
+        study_instance_uid,
+        series_instance_uid,
+        sop_instance_uid,
+        frame_numbers,
+        media_types=(media_type, )
+    )
+    assert list(result) == [content]
+    request = httpserver.requests[0]
+    expected_path = (
+        f'/studies/{study_instance_uid}'
+        f'/series/{series_instance_uid}'
+        f'/instances/{sop_instance_uid}'
+        f'/frames/{frame_list}'
+    )
+    assert request.path == expected_path
+    assert request.accept_mimetypes[0][0][:36] == headers['content-type'][:36]
+
 
 def test_retrieve_instance_frames_rendered_jpeg(httpserver, client, cache_dir):
     cache_filename = str(cache_dir.joinpath('retrieve_instance_pixeldata.jpg'))