From 0989787127bf0965ebdeeedabe6bef568d3f608f Mon Sep 17 00:00:00 2001 From: scaramallion Date: Sat, 23 Apr 2022 15:38:48 +1000 Subject: [PATCH] [MRG] Backport fix for receiving chunked datasets (#767) --- docs/changelog/index.rst | 1 + docs/changelog/v2.0.2.rst | 10 ++++++ docs/conf.py | 10 +++--- docs/index.rst | 1 + pynetdicom/apps/qrscp/db.py | 5 ++- pynetdicom/apps/tests/test_qrscp_db.py | 6 +++- pynetdicom/dimse_messages.py | 1 + pynetdicom/dimse_primitives.py | 44 +++++++++++++----------- pynetdicom/events.py | 3 ++ pynetdicom/sop_class.py | 2 +- pynetdicom/tests/test_ae.py | 6 +++- pynetdicom/tests/test_assoc_n.py | 4 +-- pynetdicom/tests/test_dimse_n.py | 16 ++++++--- pynetdicom/tests/test_events.py | 7 ++++ pynetdicom/tests/test_pdu_items.py | 5 +++ pynetdicom/tests/test_primitives.py | 6 ++++ pynetdicom/tests/test_service_storage.py | 3 ++ pynetdicom/tests/test_validators.py | 5 +++ 18 files changed, 99 insertions(+), 36 deletions(-) create mode 100644 docs/changelog/v2.0.2.rst diff --git a/docs/changelog/index.rst b/docs/changelog/index.rst index 5fc8513c1b..749fe990cc 100644 --- a/docs/changelog/index.rst +++ b/docs/changelog/index.rst @@ -7,6 +7,7 @@ Release Notes .. toctree:: :maxdepth: 1 + v2.0.2 v2.0.1 v2.0.0 v1.5.7 diff --git a/docs/changelog/v2.0.2.rst b/docs/changelog/v2.0.2.rst new file mode 100644 index 0000000000..923a2b5a1e --- /dev/null +++ b/docs/changelog/v2.0.2.rst @@ -0,0 +1,10 @@ +.. _v2.0.2: + +2.0.2 +===== + +Fixes +..... + +* Fixed datasets not transferring correctly when using + :attr:`~pynetdicom._config.STORE_RECV_CHUNKED_DATASET` (:issue:`765`) diff --git a/docs/conf.py b/docs/conf.py index 84b6a31231..d0ef7583ef 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -123,9 +123,9 @@ master_doc = "index" # General information about the project. -project = u"pynetdicom" +project = "pynetdicom" year = datetime.now().strftime("%Y") -copyright = u"2018-{}, pynetdicom contributors".format(year) +copyright = "2018-{}, pynetdicom contributors".format(year) # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the @@ -266,8 +266,8 @@ ( "index", "pynetdicom.tex", - u"pynetdicom Documentation", - u"pynetdicom contributors", + "pynetdicom Documentation", + "pynetdicom contributors", "manual", ), ] @@ -316,5 +316,5 @@ def setup(app): # The following is used by sphinx.ext.linkcode to provide links to github linkcode_resolve = make_linkcode_resolve( "pynetdicom", - u"https://github.com/pydicom/pynetdicom/blob/{revision}/{package}/{path}#L{lineno}", + "https://github.com/pydicom/pynetdicom/blob/{revision}/{package}/{path}#L{lineno}", ) diff --git a/docs/index.rst b/docs/index.rst index ff47aa91e0..b38415756e 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -119,6 +119,7 @@ Applications Release Notes ============= +* :doc:`v2.0.2 ` * :doc:`v2.0.1 ` * :doc:`v2.0.0 ` * :doc:`v1.5.7 ` diff --git a/pynetdicom/apps/qrscp/db.py b/pynetdicom/apps/qrscp/db.py index 7ecef34171..ea235197d7 100644 --- a/pynetdicom/apps/qrscp/db.py +++ b/pynetdicom/apps/qrscp/db.py @@ -205,7 +205,7 @@ def add_instance(ds, session, fpath=None): assert len(value) <= max_len else: - assert -(2 ** 31) <= value <= 2 ** 31 - 1 + assert -(2**31) <= value <= 2**31 - 1 setattr(instance, attr, value) @@ -605,6 +605,9 @@ def _search_uid_list(elem, session, query=None): if not query: query = session.query(Instance) + if elem.VM == 1: + return query.filter(attr == elem.value) + return query.filter(attr.in_(elem.value)) diff --git a/pynetdicom/apps/tests/test_qrscp_db.py b/pynetdicom/apps/tests/test_qrscp_db.py index 0cbae6558e..19a9ff6801 100644 --- a/pynetdicom/apps/tests/test_qrscp_db.py +++ b/pynetdicom/apps/tests/test_qrscp_db.py @@ -16,7 +16,7 @@ except ImportError: HAVE_SQLALCHEMY = False -from pydicom import dcmread +from pydicom import dcmread, config as PYD_CONFIG import pydicom.config from pydicom.dataset import Dataset from pydicom.tag import Tag @@ -30,6 +30,10 @@ from pynetdicom.apps.qrscp import db +if hasattr(PYD_CONFIG, "settings"): + PYD_CONFIG.settings.reading_validation_mode = 0 + + TEST_DIR = os.path.dirname(__file__) DATA_DIR = os.path.join(TEST_DIR, "../", "../", "tests", "dicom_files") DATASETS = { diff --git a/pynetdicom/dimse_messages.py b/pynetdicom/dimse_messages.py index a307cfcd40..452fdbd9c6 100644 --- a/pynetdicom/dimse_messages.py +++ b/pynetdicom/dimse_messages.py @@ -497,6 +497,7 @@ def decode_msg( # number of P-DATA primitives. if self._data_set_file: self._data_set_file.write(data[1:]) + self._data_set_file.file.flush() else: cast(BytesIO, self.data_set).write(data[1:]) diff --git a/pynetdicom/dimse_primitives.py b/pynetdicom/dimse_primitives.py index 6febb78321..df410db48e 100644 --- a/pynetdicom/dimse_primitives.py +++ b/pynetdicom/dimse_primitives.py @@ -6,7 +6,7 @@ in order for the DIMSE messages/primitives to be created correctly. """ -from collections.abc import MutableSequence +from collections.abc import Sequence from io import BytesIO import logging from pathlib import Path @@ -21,11 +21,13 @@ if TYPE_CHECKING: # pragma: no cover + from io import BufferedWriter from typing import Protocol # Python 3.8+ class NTF(Protocol): # Protocol for a NamedTemporaryFile name: str + file: BufferedWriter def write(self, data: bytes) -> bytes: ... @@ -67,7 +69,7 @@ class DIMSEPrimitive: _action_type_id: Optional[int] = None _affected_sop_class_uid: Optional[UID] = None _affected_sop_instance_uid: Optional[UID] = None - _attribute_identifier_list: Optional[List[BaseTag]] = None + _attribute_identifier_list: Union[None, BaseTag, List[BaseTag]] = None _dataset: Optional[BytesIO] = None _event_type_id: Optional[int] = None _message_id: Optional[int] = None @@ -206,7 +208,7 @@ def MessageID(self) -> Optional[int]: def MessageID(self, value: Optional[int]) -> None: """Set the *Message ID*.""" if isinstance(value, int): - if 0 <= value < 2 ** 16: + if 0 <= value < 2**16: self._message_id = value else: raise ValueError("Message ID must be between 0 and 65535, inclusive") @@ -230,7 +232,7 @@ def MessageIDBeingRespondedTo(self) -> Optional[int]: def MessageIDBeingRespondedTo(self, value: Optional[int]) -> None: """Set the *Message ID Being Responded To*.""" if isinstance(value, int): - if 0 <= value < 2 ** 16: + if 0 <= value < 2**16: self._message_id_being_responded_to = value else: raise ValueError( @@ -591,7 +593,7 @@ def MoveOriginatorMessageID(self, value: Optional[int]) -> None: """ # Fix for peers sending a value consisting of nulls if isinstance(value, int): - if 0 <= value < 2 ** 16: + if 0 <= value < 2**16: self._move_originator_message_id = value else: raise ValueError( @@ -1251,7 +1253,7 @@ def MessageIDBeingRespondedTo(self) -> Optional[int]: def MessageIDBeingRespondedTo(self, value: Optional[int]) -> None: """Set the *Message ID Being Responded To*.""" if isinstance(value, int): - if 0 <= value < 2 ** 16: + if 0 <= value < 2**16: self._message_id_being_responded_to = value else: raise ValueError( @@ -1502,13 +1504,13 @@ def AffectedSOPInstanceUID(self, value: OptionalUIDType) -> None: self._AffectedSOPInstanceUID = value # type: ignore @property - def AttributeIdentifierList(self) -> Optional[List[BaseTag]]: + def AttributeIdentifierList(self) -> Optional[Union[BaseTag, List[BaseTag]]]: """Get or set the *Attribute Identifier List* as a :class:`list` of :class:`~pydicom.tag.BaseTag`. Parameters ---------- - list of pydicom.tag.BaseTag + pydicom.tag.BaseTag or list of pydicom.tag.BaseTag The value to use for the *Attribute Identifier List* parameter. A list of pydicom :class:`pydicom.tag.BaseTag` instances or any values acceptable for creating them. @@ -1524,20 +1526,22 @@ def AttributeIdentifierList( self._attribute_identifier_list = None return - # Singleton tags get put in a list - if not isinstance(value, (list, MutableSequence)): - value = [value] - - # Empty list -> None - if not value: - self._attribute_identifier_list = None - return - try: - # Convert each item in list to pydicom Tag - self._attribute_identifier_list = [Tag(tag) for tag in value] + if isinstance(value, Sequence): + if not value: + self._attribute_identifier_list = None + + if len(value) == 1: + self._attribute_identifier_list = Tag(value[0]) + else: + self._attribute_identifier_list = [Tag(tag) for tag in value] + else: + self._attribute_identifier_list = Tag(value) except (TypeError, ValueError): - raise ValueError("Attribute Identifier List must be a list of pydicom Tags") + raise ValueError( + "Attribute Identifier List must be convertible to a pydicom " + "BaseTag or list of BaseTag" + ) @property def AttributeList(self) -> Optional[BytesIO]: diff --git a/pynetdicom/events.py b/pynetdicom/events.py index 0bf7eb3f34..57b1e2fc07 100644 --- a/pynetdicom/events.py +++ b/pynetdicom/events.py @@ -549,6 +549,9 @@ def attribute_identifiers(self) -> List[BaseTag]: if attr_list is None: return [] + if not isinstance(attr_list, list): + return [attr_list] + return attr_list except AttributeError: pass diff --git a/pynetdicom/sop_class.py b/pynetdicom/sop_class.py index fad0e04275..d05790ce72 100644 --- a/pynetdicom/sop_class.py +++ b/pynetdicom/sop_class.py @@ -117,7 +117,7 @@ def __new__(cls: Type["SOPClass"], val: str) -> "SOPClass": if isinstance(val, SOPClass): return val - return super(SOPClass, cls).__new__(cls, val) + return cast("SOPClass", super().__new__(cls, val)) def __getattribute__(self, name: str) -> Any: return super(SOPClass, self).__getattribute__(name) diff --git a/pynetdicom/tests/test_ae.py b/pynetdicom/tests/test_ae.py index 997b0cbbbc..c5b626d634 100644 --- a/pynetdicom/tests/test_ae.py +++ b/pynetdicom/tests/test_ae.py @@ -8,7 +8,7 @@ import pytest -from pydicom import read_file +from pydicom import read_file, config as PYD_CONFIG from pydicom.dataset import Dataset from pydicom.uid import UID, ImplicitVRLittleEndian @@ -29,6 +29,10 @@ from pynetdicom.transport import AssociationServer, RequestHandler +if hasattr(PYD_CONFIG, "settings"): + PYD_CONFIG.settings.reading_validation_mode = 0 + + # debug_logger() diff --git a/pynetdicom/tests/test_assoc_n.py b/pynetdicom/tests/test_assoc_n.py index 8b304d4697..0bf620e5fe 100644 --- a/pynetdicom/tests/test_assoc_n.py +++ b/pynetdicom/tests/test_assoc_n.py @@ -1059,7 +1059,7 @@ def handle(event): ds.Status = 0xFFF0 ds.ErrorComment = "Some comment" ds.ErrorID = 12 - ds.AttributeIdentifierList = [0x00100020] + ds.AttributeIdentifierList = 0x00100020 return ds, None self.ae = ae = AE() @@ -1081,7 +1081,7 @@ def handle(event): assert status.Status == 0xFFF0 assert status.ErrorComment == "Some comment" assert status.ErrorID == 12 - assert status.AttributeIdentifierList == [0x00100020] + assert status.AttributeIdentifierList == 0x00100020 assert ds is None assoc.release() assert assoc.is_released diff --git a/pynetdicom/tests/test_dimse_n.py b/pynetdicom/tests/test_dimse_n.py index 3ae23ccf25..2237e288b2 100644 --- a/pynetdicom/tests/test_dimse_n.py +++ b/pynetdicom/tests/test_dimse_n.py @@ -9,9 +9,10 @@ import pytest +from pydicom import config as PYD_CONFIG from pydicom.dataset import Dataset from pydicom.dataelem import DataElement -from pydicom.tag import Tag +from pydicom.tag import Tag, BaseTag from pydicom.uid import UID from pynetdicom import _config @@ -63,6 +64,11 @@ n_create_rsp_ds, ) + +if hasattr(PYD_CONFIG, "settings"): + PYD_CONFIG.settings.reading_validation_mode = 0 + + LOGGER = logging.getLogger("pynetdicom") LOGGER.setLevel(logging.CRITICAL) @@ -375,14 +381,14 @@ def test_assignment(self): Tag(0x7FE0, 0x0010), ] == primitive.AttributeIdentifierList primitive.AttributeIdentifierList = [(0x7FE0, 0x0010)] - assert [Tag(0x7FE0, 0x0010)] == primitive.AttributeIdentifierList + assert Tag(0x7FE0, 0x0010) == primitive.AttributeIdentifierList primitive.AttributeIdentifierList = (0x7FE0, 0x0010) - assert [Tag(0x7FE0, 0x0010)] == primitive.AttributeIdentifierList + assert Tag(0x7FE0, 0x0010) == primitive.AttributeIdentifierList elem = DataElement((0x0000, 0x0005), "AT", [Tag(0x0000, 0x1000)]) - assert isinstance(elem.value, MutableSequence) + assert isinstance(elem.value, BaseTag) primitive.AttributeIdentifierList = elem.value - assert [Tag(0x0000, 0x1000)] == primitive.AttributeIdentifierList + assert Tag(0x0000, 0x1000) == primitive.AttributeIdentifierList # MessageID primitive.MessageID = 11 diff --git a/pynetdicom/tests/test_events.py b/pynetdicom/tests/test_events.py index 3b7503b6b4..e0fda7fa37 100644 --- a/pynetdicom/tests/test_events.py +++ b/pynetdicom/tests/test_events.py @@ -479,6 +479,13 @@ def test_attr_identifiers(self): assert isinstance(tags[1], BaseTag) assert tags[1] == 0x00100020 + request.AttributeIdentifierList = 0x00100010 + event = Event( + None, evt.EVT_N_GET, {"request": request, "context": self.context.as_tuple} + ) + tags = event.attribute_identifiers + assert tags == [0x00100010] + def test_action_type(self): """Test with action_type.""" request = N_ACTION() diff --git a/pynetdicom/tests/test_pdu_items.py b/pynetdicom/tests/test_pdu_items.py index 68a20ca6c6..4d42ddefcb 100644 --- a/pynetdicom/tests/test_pdu_items.py +++ b/pynetdicom/tests/test_pdu_items.py @@ -6,6 +6,7 @@ import pytest +from pydicom import config as PYD_CONFIG from pydicom.uid import UID from pynetdicom import _config, debug_logger @@ -85,6 +86,10 @@ ) +if hasattr(PYD_CONFIG, "settings"): + PYD_CONFIG.settings.reading_validation_mode = 0 + + # debug_logger() diff --git a/pynetdicom/tests/test_primitives.py b/pynetdicom/tests/test_primitives.py index 53afce7dd6..fe4dd91ba8 100644 --- a/pynetdicom/tests/test_primitives.py +++ b/pynetdicom/tests/test_primitives.py @@ -4,6 +4,7 @@ import pytest +from pydicom import config as PYD_CONFIG from pydicom.uid import UID from pynetdicom import _config @@ -26,6 +27,11 @@ from pynetdicom.presentation import PresentationContext from pynetdicom.utils import pretty_bytes + +if hasattr(PYD_CONFIG, "settings"): + PYD_CONFIG.settings.reading_validation_mode = 0 + + LOGGER = logging.getLogger("pynetdicom") LOGGER.setLevel(logging.CRITICAL) diff --git a/pynetdicom/tests/test_service_storage.py b/pynetdicom/tests/test_service_storage.py index 3fd41ad55d..682ef983c7 100644 --- a/pynetdicom/tests/test_service_storage.py +++ b/pynetdicom/tests/test_service_storage.py @@ -472,6 +472,7 @@ def handle(event): handlers = [(evt.EVT_C_STORE, handle)] self.ae = ae = AE() + ae.maximum_pdu_size = 256 ae.add_supported_context(CTImageStorage) ae.add_requested_context(CTImageStorage) scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers) @@ -492,6 +493,8 @@ def handle(event): ds = attrs["dataset"] assert "CompressedSamples^CT1" == ds.PatientName + assert "DataSetTrailingPadding" in ds + assert len(ds.DataSetTrailingPadding) == 126 scp.shutdown() diff --git a/pynetdicom/tests/test_validators.py b/pynetdicom/tests/test_validators.py index 207ee3161d..e79371c9ff 100644 --- a/pynetdicom/tests/test_validators.py +++ b/pynetdicom/tests/test_validators.py @@ -2,11 +2,16 @@ import pytest +from pydicom import config as PYD_CONFIG from pynetdicom import _config from pynetdicom._validators import validate_ae, validate_ui +if hasattr(PYD_CONFIG, "settings"): + PYD_CONFIG.settings.reading_validation_mode = 0 + + AE_REFERENCE = [ ("", True, ""), ("A", True, ""),