Repository: pydicom/pynetdicom
Pull request #542 (2020-09-22 06:52:28)

[MRG] Fix Move Originator Message ID not being set correctly when Move SCP
#### Reference issue
Related to #541 

#### Tasks
- [x] Unit tests added that reproduce issue or prove feature is working
- [x] Fix or feature added
- [x] Documentation and examples updated (if relevant)
- [x] Unit tests passing and coverage at 100% after adding fix/feature



Patch:
diff --git a/docs/changelog/index.rst b/docs/changelog/index.rst
index 4957340e6..2ae0a9e49 100644
--- a/docs/changelog/index.rst
+++ b/docs/changelog/index.rst
@@ -8,6 +8,7 @@ Release Notes
    :maxdepth: 1
 
    v2.0.0
+   v1.5.3
    v1.5.2
    v1.5.1
    v1.5.0
diff --git a/docs/changelog/v1.5.3.rst b/docs/changelog/v1.5.3.rst
new file mode 100644
index 000000000..d177a17b2
--- /dev/null
+++ b/docs/changelog/v1.5.3.rst
@@ -0,0 +1,10 @@
+.. _v1.5.3:
+
+1.5.3
+=====
+
+Fixes
+.....
+
+* Fixed not processing multiple PDUs received when acting as the association
+  requestor with SSL/TLS (:pr:`528`)
diff --git a/docs/changelog/v2.0.0.rst b/docs/changelog/v2.0.0.rst
index 3e0c80a89..ec5e8f6fc 100644
--- a/docs/changelog/v2.0.0.rst
+++ b/docs/changelog/v2.0.0.rst
@@ -6,8 +6,10 @@
 Fixes
 .....
 
-* Fixed not processing multiple PDUs received when acting as the association 
+* Fixed not processing multiple PDUs received when acting as the association
   requestor with SSL/TLS (:pr:`528`)
+* Fixed not using the correct *Move Originator Message ID* when sending
+  C-STORE-RQs when acting as a Move SCP (:issue:`541`)
 
 Enhancements
 ............
@@ -20,7 +22,10 @@ Enhancements
 * Added :attr:`~pynetdicom._config.STORE_SEND_CHUNKED_DATASET` config option
   to allow the use of a chunked read when a file path is passed to
   :meth:`~pynetdicom.association.Association.send_c_store` in order to
-  reduce the memory needed for sending large dataset (:issue:`517`)
+  reduce the memory needed for sending datasets (:issue:`517`)
+* Added :attr:`~pynetdicom._config.STORE_RECV_CHUNKED_DATASET` config option
+  to allow storing received C-STORE datasets directly to file,
+  reducing the memory needed when receiving datasets (:issue:`517`)
 
 Changes
 .......
diff --git a/pynetdicom/_config.py b/pynetdicom/_config.py
index 9bd19b66f..69e283905 100644
--- a/pynetdicom/_config.py
+++ b/pynetdicom/_config.py
@@ -110,14 +110,15 @@ If ``True``, then when using
 :meth:`~pynetdicom.association.Association.send_c_store` with a file path to
 a DICOM dataset, don't decode the dataset and instead send the raw encoded
 data (without the File Meta Information) in chunks of no larger than
-maximum PDU size allowed by the peer. This should
-minimise the amount of memory required when:
+maximum PDU size allowed by the peer. This should minimise the amount of
+memory required when:
 
 * Sending large datasets
 * Sending many datasets concurrently
 
-It is not possible to convert the dataset without loading it into memory
-and so an exact matching accepted presentation context will be required.
+As it's not possible to change the dataset encoding without loading it into
+memory, an exact matching accepted presentation context will be required.
+
 Default: ``False``.
 
 Examples
@@ -132,12 +133,12 @@ STORE_RECV_CHUNKED_DATASET = False
 
 .. versionadded:: 2.0
 
-If ``True``, then when receiving C-STORE primitives as an SCP, don't encode
-the dataset and instead receive the raw data in chunks of no larger
-than the maximum PDU size allowed by the peer. The dataset is written to a
-temporary file on disk (with the File Meta Information). The path of the
-dataset is available in the C-STORE event handler on the property
-``dataset_path``. This should minimise the amount of memory required when:
+If ``True``, then when receiving C-STORE requests as an SCP, don't decode
+the dataset and instead write the raw data to a temporary file in the DICOM
+File Format. The path to the dataset is available to the ``evt.EVT_C_STORE``
+handler using the  :attr:`Event.dataset_path
+<pynetdicom.events.Event.dataset_path>` attribute. This should minimise the
+amount of memory required when:
 
 * Receiving large datasets
 * Receiving many datasets concurrently
diff --git a/pynetdicom/_handlers.py b/pynetdicom/_handlers.py
index 4002dc693..5bf1536f7 100644
--- a/pynetdicom/_handlers.py
+++ b/pynetdicom/_handlers.py
@@ -2593,6 +2593,9 @@ def doc_handle_store(event, *args):
           a deferred read when decoding data, if the decode fails the returned
           :class:`~pydicom.dataset.Dataset` will only raise an exception at the
           time of use.
+        * :attr:`~pynetdicom.events.Event.dataset_path`: when
+          :attr:`~pynetdicom._config.STORE_RECV_CHUNKED_DATASET` is ``True``,
+          this is the path to the received dataset as :class:`pathlib.Path`.
         * :attr:`~pynetdicom.events.Event.file_meta`: a
           :class:`~pydicom.dataset.Dataset` containing DICOM
           conformant File Meta Information that can be used with the decoded
diff --git a/pynetdicom/dimse_messages.py b/pynetdicom/dimse_messages.py
index c05043e6b..9a8ccd1e6 100644
--- a/pynetdicom/dimse_messages.py
+++ b/pynetdicom/dimse_messages.py
@@ -249,15 +249,14 @@ class DIMSEMessage(object):
         ----------
         primitive : pdu_primitives.P_DATA
             The P-DATA service primitive to be decoded into a DIMSE message.
+        assoc : association.Association, optional
+            The association processing the message. This is required when:
 
-        assoc : association.Association
-            The Association that produced the P-DATA service primitive. This
-            is only necessary when:
+            * :attr:`~pynetdicom._config.STORE_RECV_CHUNKED_DATASET` is
+              ``True``
+            * The P-DATA primitive contains part of a C-STORE-RQ message
 
-            * :attr:`~pynetdicom._config.STORE_RECV_CHUNKED_DATASET` is ``True``
-            * The P_DATA primitive is a part of a C_STORE_RQ DIMSE message
-
-            In this case the Association is consulted for its accepted
+            In this case the association is consulted for its accepted
             transfer syntax, which is included in the File Meta Information
             of the stored dataset.
 
@@ -330,9 +329,10 @@ class DIMSEMessage(object):
                         #   has been completely decoded
                         return True
 
+                    # Data Set is present
                     if (
-                        _config.STORE_RECV_CHUNKED_DATASET and
-                        isinstance(self, C_STORE_RQ)
+                        _config.STORE_RECV_CHUNKED_DATASET
+                        and isinstance(self, C_STORE_RQ)
                     ):
                         # delete=False is a workaround for Windows
                         # Setting delete=True prevents us from re-opening
@@ -344,16 +344,18 @@ class DIMSEMessage(object):
                             suffix=".dcm"
                         )
                         self._data_set_path = Path(self._data_set_file.name)
-
+                        # Write the File Meta
                         self._data_set_file.write(b'\x00' * 128)
                         self._data_set_file.write(b'DICM')
 
+                        cs = self.command_set
+                        cx = assoc._accepted_cx[context_id]
                         write_file_meta_info(
                             self._data_set_file,
                             create_file_meta(
-                                sop_class_uid=self.command_set.AffectedSOPClassUID,
-                                sop_instance_uid=self.command_set.AffectedSOPInstanceUID,
-                                transfer_syntax=assoc._accepted_cx[context_id].transfer_syntax[0],
+                                sop_class_uid=cs.AffectedSOPClassUID,
+                                sop_instance_uid=cs.AffectedSOPInstanceUID,
+                                transfer_syntax=cx.transfer_syntax[0]
                             )
                         )
 
diff --git a/pynetdicom/dsutils.py b/pynetdicom/dsutils.py
index f3a77db1d..1a0408e4d 100644
--- a/pynetdicom/dsutils.py
+++ b/pynetdicom/dsutils.py
@@ -4,17 +4,69 @@ import logging
 import zlib
 
 from pydicom import Dataset
+from pydicom.dataset import FileMetaDataset
 from pydicom.filebase import DicomBytesIO
 from pydicom.filereader import read_dataset, read_preamble
 from pydicom.filewriter import write_dataset
 
-from pynetdicom import PYNETDICOM_IMPLEMENTATION_UID, PYNETDICOM_IMPLEMENTATION_VERSION
+from pynetdicom import (
+    PYNETDICOM_IMPLEMENTATION_UID, PYNETDICOM_IMPLEMENTATION_VERSION
+)
 from pynetdicom.utils import pretty_bytes
 
 
 LOGGER = logging.getLogger('pynetdicom.dsutils')
 
 
+
+
+def create_file_meta(
+    *,
+    sop_class_uid,
+    sop_instance_uid,
+    transfer_syntax,
+    implementation_uid=PYNETDICOM_IMPLEMENTATION_UID,
+    implementation_version=PYNETDICOM_IMPLEMENTATION_VERSION,
+):
+    """Return a new file meta dataset
+
+    .. versionadded:: 2.0
+
+    Parameters
+    ----------
+    sop_class_uid : pydicom.uid.UID
+        The value for the *Media Storage SOP Class UID*.
+    sop_instance_uid : pydicom.uid.UID
+        The value for the *Media Storage SOP Instance UID*.
+    transfer_syntax : pydicom.uid.UID
+        The value for the *Transfer Syntax UID*.
+    implementation_uid : pydicom.uid.UID, optional
+        The value for the *Implementation Class UID*.
+    implementation_version : str, optional
+        The value for the *Implementation Version Name*.
+
+    Returns
+    -------
+    pydicom.dataset.FileMetaDataset
+        The File Meta dataset
+    """
+    file_meta = FileMetaDataset()
+
+    file_meta.FileMetaInformationGroupLength = 0
+    file_meta.FileMetaInformationVersion = b'\x00\x01'
+    file_meta.MediaStorageSOPClassUID = sop_class_uid
+    file_meta.MediaStorageSOPInstanceUID = sop_instance_uid
+    file_meta.TransferSyntaxUID = transfer_syntax
+    file_meta.ImplementationClassUID = implementation_uid
+    file_meta.ImplementationVersionName = implementation_version
+
+    # File Meta Information is always encoded as Explicit VR Little Endian
+    file_meta.is_little_endian = True
+    file_meta.is_implicit_VR = False
+
+    return file_meta
+
+
 def decode(bytestring, is_implicit_vr, is_little_endian, deflated=False):
     """Decode `bytestring` to a *pydicom* :class:`~pydicom.dataset.Dataset`.
 
@@ -244,56 +296,3 @@ def split_dataset(fpath):
             stop_when=_not_group_0002
         )
         return file_meta, fp.tell()
-
-
-def create_file_meta(
-    *,
-    sop_class_uid,
-    sop_instance_uid,
-    transfer_syntax,
-    group_length=0,
-    version=b'\x00\x01',
-    implementation_uid=PYNETDICOM_IMPLEMENTATION_UID,
-    implementation_version=PYNETDICOM_IMPLEMENTATION_VERSION,
-):
-    """Return a new file meta dataset
-
-    .. versionadded:: 2.0
-
-    Parameters
-    ----------
-    sop_class_uid : pydicom.uid.UID
-        The value for the MediaStorageSOPClassUID attribute.
-    sop_instance_uid : pydicom.uid.UID
-        The value for the MediaStorageSOPInstanceUID attribute.
-    transfer_syntax : pydicom.uid.UID
-        The value for the TransferSyntax attribute.
-    group_length : int
-        The value for the FileMetaInformationGroupLength attribute.
-    version : bytes
-        The value for the FileMetaInformationVersion attribute.
-    implementation_uid : pydicom.uid.UID
-        The value for the ImplementationClassUID attribute.
-    implementation_version : str
-        The value for the ImplementationVersionName attribute.
-
-    Returns
-    -------
-    pydicom.dataset.Dataset
-        The File Meta dataset
-    """
-    file_meta = Dataset()
-
-    file_meta.FileMetaInformationGroupLength = group_length
-    file_meta.FileMetaInformationVersion = version
-    file_meta.MediaStorageSOPClassUID = sop_class_uid
-    file_meta.MediaStorageSOPInstanceUID = sop_instance_uid
-    file_meta.TransferSyntaxUID = transfer_syntax
-    file_meta.ImplementationClassUID = implementation_uid
-    file_meta.ImplementationVersionName = implementation_version
-
-    # File Meta Information is always encoded as Explicit VR Little Endian
-    file_meta.is_little_endian = True
-    file_meta.is_implicit_VR = False
-
-    return file_meta
diff --git a/pynetdicom/events.py b/pynetdicom/events.py
index 2c0fe7809..6c301d28b 100644
--- a/pynetdicom/events.py
+++ b/pynetdicom/events.py
@@ -5,11 +5,14 @@ the state machine events.
 from collections import namedtuple
 from datetime import datetime
 import inspect
+from io import BytesIO
 import logging
 import sys
 
 from pydicom.dataset import Dataset
+from pydicom.filereader import dcmread
 
+from pynetdicom import _config
 from pynetdicom.dsutils import decode, create_file_meta
 
 
@@ -418,16 +421,29 @@ class Event(object):
             "The corresponding event is not a C-STORE request and has no "
             "'Data Set' parameter"
         )
+        try:
+            return dcmread(self.dataset_path)
+        except (TypeError, AttributeError):
+            pass
+
         return self._get_dataset("DataSet", msg)
 
     @property
     def dataset_path(self):
+        """Return the path to the dataset when
+        :attr:`~pynetdicom._config.STORE_RECV_CHUNKED_DATASET` is ``True``.
+
+        Returns
+        -------
+        pathlib.Path
+            The path to the dataset.
+        """
         try:
             dataset_path = self.request._dataset_path
         except AttributeError:
             msg = (
                 "The corresponding event is either not a C-STORE request or "
-                "'STORE_RECV_CHUNKED_DATASET' is not set."
+                "'STORE_RECV_CHUNKED_DATASET' is not True."
             )
             raise AttributeError(msg)
 
@@ -596,6 +612,7 @@ class Event(object):
         """
         try:
             bytestream = getattr(self.request, attr)
+
             # If no change in encoded data then return stored decode
             if self._hash == hash(bytestream):
                 return self._decoded
diff --git a/pynetdicom/service_class.py b/pynetdicom/service_class.py
index 1f58c1d7d..3ca81c5dd 100644
--- a/pynetdicom/service_class.py
+++ b/pynetdicom/service_class.py
@@ -2214,7 +2214,7 @@ class QueryRetrieveServiceClass(ServiceClass):
                         dataset,
                         msg_id=msg_id,
                         originator_aet=self.ae.ae_title,
-                        originator_id=1
+                        originator_id=req.MessageID
                     )
 
                     store_status_int = store_status.Status


Test patch:
diff --git a/pynetdicom/tests/test_events.py b/pynetdicom/tests/test_events.py
index 5dfa52a99..57656d0cd 100644
--- a/pynetdicom/tests/test_events.py
+++ b/pynetdicom/tests/test_events.py
@@ -192,7 +192,7 @@ class TestEvent(object):
 
         msg = (
             r"The corresponding event is either not a C-STORE request or "
-            r"'STORE_RECV_CHUNKED_DATASET' is not set."
+            r"'STORE_RECV_CHUNKED_DATASET' is not True."
         )
         with pytest.raises(AttributeError, match=msg):
             event.dataset_path
diff --git a/pynetdicom/tests/test_service_qr.py b/pynetdicom/tests/test_service_qr.py
index b31e57580..2254c7458 100644
--- a/pynetdicom/tests/test_service_qr.py
+++ b/pynetdicom/tests/test_service_qr.py
@@ -4564,6 +4564,10 @@ class TestQRMoveServiceClass(object):
             yield 0xFF00, self.ds
 
         def handle_store(event):
+            attrs['originator_msg_id'] = event.request.MoveOriginatorMessageID
+            attrs['originator_aet'] = (
+                event.request.MoveOriginatorApplicationEntityTitle
+            )
             return 0x0000
 
         handlers = [(evt.EVT_C_MOVE, handle), (evt.EVT_C_STORE, handle_store)]
@@ -4579,7 +4583,11 @@ class TestQRMoveServiceClass(object):
         ae.dimse_timeout = 5
         assoc = ae.associate('localhost', 11112)
         assert assoc.is_established
-        result = assoc.send_c_move(self.query, b'TESTMOVE', PatientRootQueryRetrieveInformationModelMove)
+        result = assoc.send_c_move(
+            self.query, b'TESTMOVE',
+            PatientRootQueryRetrieveInformationModelMove,
+            msg_id=1234
+        )
         status, identifier = next(result)
         assert status.Status == 0xFF00
         assert identifier is None
@@ -4594,6 +4602,9 @@ class TestQRMoveServiceClass(object):
         assert isinstance(req, C_MOVE)
         assert req.MoveDestination == b'TESTMOVE'
 
+        assert attrs['originator_msg_id'] == 1234
+        assert attrs['originator_aet'] == b'PYNETDICOM'
+
         scp.shutdown()
 
     def test_scp_handler_identifier(self):
diff --git a/pynetdicom/tests/test_service_storage.py b/pynetdicom/tests/test_service_storage.py
index 9c6375bc9..5379f7676 100644
--- a/pynetdicom/tests/test_service_storage.py
+++ b/pynetdicom/tests/test_service_storage.py
@@ -430,9 +430,7 @@ class TestStorageServiceClass(object):
                 == DATASET.file_meta.MediaStorageSOPInstanceUID
             )
 
-            # `dataset` property empty
-            assert event.dataset == Dataset()
-
+            attrs['dataset'] = event.dataset
             attrs['dataset_path'] = dataset_path
             return 0x0000
 
@@ -459,6 +457,9 @@ class TestStorageServiceClass(object):
         with pytest.raises(FileNotFoundError):
             dataset_path.open("rb")
 
+        ds = attrs['dataset']
+        assert "CompressedSamples^CT1" == ds.PatientName
+
         scp.shutdown()
 
     def test_scp_handler_dataset_path_windows_unlink(self, monkeypatch):