Fix storage of image instances via file client

Author: hackermd

src/dicomweb_client/file.py

@@ -1997,16 +1997,27 @@ def insert_instances(
                         transfer_syntax_uid=transfer_syntax_uid
                     )
                     dcmwrite(b, ds, write_like_original=False)
+                    # The data set needs to be read back (at least partially)
+                    # to determine the offset of the Pixel Data element. This
+                    # is required to either read or build the Basic Offset Table
+                    # for image instances to allow for fast retrieval of frames.
+                    fp.seek(0)
+                    # One needs to specify at least one tag to satisfy mypy.
+                    tag = tag_for_keyword('PatientID')
+                    dcmread(
+                        fp,
+                        specific_tags=[tag],  # type: ignore
+                        stop_before_pixels=True
+                    )
+                    pixel_data_offset = fp.tell()
+
                     pixel_data_element: Union[DataElement, None] = None
                     for pixel_data_tag in _PIXEL_DATA_TAGS:
                         try:
                             pixel_data_element = ds[pixel_data_tag]
                         except KeyError:
                             continue
                     if pixel_data_element is not None:
-                        pixel_data_offset = pixel_data_element.file_tell
-                        if pixel_data_offset is None:
-                            continue
                         fp.seek(pixel_data_offset, 0)
                         first_frame_offset, bot = _get_frame_offsets(
                             fp,
@@ -2017,7 +2028,8 @@ def insert_instances(
                                 np.product([
                                     ds.Rows,
                                     ds.Columns,
-                                    ds.SamplesPerPixel])
+                                    ds.SamplesPerPixel
+                                ])
                             ),
                             transfer_syntax_uid=ds.file_meta.TransferSyntaxUID,
                             bits_allocated=ds.BitsAllocated
@@ -2028,14 +2040,14 @@ def insert_instances(
 
                     fp.seek(0)
                     file_content = fp.read()
+
                 instances[sop_instance_uid] = (
                     *instance_metadata,
                     str(ds.file_meta.TransferSyntaxUID),
                     str(rel_file_path),
                     first_frame_offset,
                     bot,
                 )
-
                 file_path = self.base_dir.joinpath(rel_file_path)
                 successes.append((ds, file_path, file_content))
             except Exception as error:
@@ -2899,6 +2911,7 @@ def retrieve_instance_rendered(
             frame_index=frame_index,
             transfer_syntax_uid=transfer_syntax_uid
         )
+        image_file_pointer.close()
 
         # TODO: ICC Profile
         codec_name, codec_kwargs = self._get_image_codec_parameters(
@@ -3187,6 +3200,8 @@ def iter_instance_frames(
             else:
                 yield frame
 
+        image_file_pointer.close()
+
     def retrieve_instance_frames(
         self,
         study_instance_uid: str,
@@ -3313,6 +3328,7 @@ def retrieve_instance_frames(
             else:
                 retrieved_frames.append(frame)
 
+        image_file_pointer.close()
         return retrieved_frames
 
     def retrieve_instance_frames_rendered(
@@ -3383,6 +3399,7 @@ def retrieve_instance_frames_rendered(
             frame_index=frame_index,
             transfer_syntax_uid=transfer_syntax_uid
         )
+        image_file_pointer.close()
 
         # TODO: ICC Profile
         codec_name, codec_kwargs = self._get_image_codec_parameters(
@@ -3573,6 +3590,9 @@ def store_instances(
         response = Dataset()
         response.RetrieveURL = None
 
+        if len(successes) == 0 and len(failures) == 0:
+            raise RuntimeError('Failed to store instances.')
+
         if len(successes) > 0:
             response.ReferencedSOPSequence = []
             for ds, file_path, file_content in successes:

tests/test_file.py

@@ -1,5 +1,11 @@
-from pydicom.dataset import Dataset
+import numpy as np
 import pytest
+from pydicom.dataset import Dataset, FileMetaDataset
+from pydicom.uid import (
+    ExplicitVRLittleEndian,
+    generate_uid,
+    VLWholeSlideMicroscopyImageStorage,
+)
 
 
 STUDY_ATTRIBUTES = {
@@ -274,6 +280,7 @@ def test_retrieve_instance_frames(file_client):
 
     for test_frame in frames:
         assert isinstance(test_frame, bytes)
+        assert len(test_frame) > 0
 
 
 def test_retrieve_instance_frames_rendered(file_client):
@@ -285,3 +292,43 @@ def test_retrieve_instance_frames_rendered(file_client):
         media_types=('image/png', )
     )
     assert isinstance(frame, bytes)
+    assert len(frame) > 0
+
+
+def test_store_instances(file_client):
+    dataset = Dataset()
+    dataset.PatientID = None
+    dataset.PatientSex = None
+    dataset.PatientBirthDate = None
+    dataset.StudyInstanceUID = generate_uid()
+    dataset.StudyID = None
+    dataset.StudyDate = None
+    dataset.StudyTime = None
+    dataset.ReferringPhysicianName = ''
+    dataset.SeriesInstanceUID = generate_uid()
+    dataset.SeriesNumber = 1
+    dataset.Modality = 'SM'
+    dataset.AccessionNumber = None
+    dataset.SOPInstanceUID = generate_uid()
+    dataset.SOPClassUID = VLWholeSlideMicroscopyImageStorage
+    dataset.InstanceNumber = 1
+    dataset.Rows = 10
+    dataset.Columns = 10
+    dataset.SamplesPerPixel = 3
+    dataset.BitsAllocated = 8
+    dataset.BitsStored = 8
+    dataset.HighBit = 7
+    dataset.PixelData = np.zeros(
+        (dataset.Rows, dataset.Columns, dataset.SamplesPerPixel),
+        dtype=np.dtype(f'uint{dataset.BitsAllocated}')
+    ).tobytes()
+    dataset.file_meta = FileMetaDataset()
+    dataset.file_meta.TransferSyntaxUID = ExplicitVRLittleEndian
+
+    response = file_client.store_instances([dataset])
+    assert hasattr(response, 'ReferencedSOPSequence')
+    assert not hasattr(response, 'FailedSOPSequence')
+    assert len(response.ReferencedSOPSequence) == 1
+    ref_item = response.ReferencedSOPSequence[0]
+    assert ref_item.ReferencedSOPInstanceUID == dataset.SOPInstanceUID
+    assert ref_item.ReferencedSOPClassUID == dataset.SOPClassUID