diff --git a/.github/workflows/merge-pytest.yml b/.github/workflows/merge-pytest.yml index 9fdc64ec9..a2dcf1bca 100644 --- a/.github/workflows/merge-pytest.yml +++ b/.github/workflows/merge-pytest.yml @@ -13,7 +13,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.10', '3.11', '3.12'] + python-version: ['3.10', '3.11', '3.12', '3.13'] steps: - uses: actions/checkout@v4 @@ -42,7 +42,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.10', '3.11', '3.12'] + python-version: ['3.10', '3.11', '3.12', '3.13'] steps: - uses: actions/checkout@v4 @@ -105,7 +105,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.10', '3.11', '3.12'] + python-version: ['3.10', '3.11', '3.12', '3.13'] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/pr-pytest.yml b/.github/workflows/pr-pytest.yml index 37327bc8a..7f1b9e0f0 100644 --- a/.github/workflows/pr-pytest.yml +++ b/.github/workflows/pr-pytest.yml @@ -107,7 +107,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.10', '3.11', '3.12'] + python-version: ['3.10', '3.11', '3.12', '3.13'] steps: - uses: actions/checkout@v4 diff --git a/docs/changelog/v2.2.0.rst b/docs/changelog/v2.2.0.rst new file mode 100644 index 000000000..e7152d5ef --- /dev/null +++ b/docs/changelog/v2.2.0.rst @@ -0,0 +1,11 @@ +.. _v2.2.0: + +2.2.0 +===== + +Changes +....... + +* Move and Get SCPs are now limited to a maximum of 65535 matches (:issue:`982`) +* Minimum supported pydicom version is 3.0 (:issue:`981`) +* Added support for Python 3.13 diff --git a/docs/examples/qr_get.rst b/docs/examples/qr_get.rst index bb39aa118..8e7607e94 100644 --- a/docs/examples/qr_get.rst +++ b/docs/examples/qr_get.rst @@ -3,7 +3,7 @@ Query/Retrieve (Get) Service Examples The DICOM :dcm:`Query/Retrieve Service ` provides a mechanism for a service user to query and retrieve the SOP Instances -managed by a QR SCP. The QR (Get) SOP classes allow an SCU to receive SOP +managed by a QR SCP. The QR (Get) SOP classes allow an SCU to receive up to 65535 SOP Instances that match the requested query. This is accomplished through the DIMSE C-GET and C-STORE services. Both query and retrieval occur over the same association, with the SCP of the Query/Retrieve diff --git a/docs/examples/qr_move.rst b/docs/examples/qr_move.rst index ee952004d..b4295df2b 100644 --- a/docs/examples/qr_move.rst +++ b/docs/examples/qr_move.rst @@ -4,7 +4,7 @@ Query/Retrieve (Move) Service Examples The DICOM :dcm:`Query/Retrieve Service ` provides a mechanism for a service user to query and retrieve the SOP Instances managed by a QR SCP. The QR (Move) SOP classes allow an SCU to request an SCP -send matching SOP Instances to a known Storage SCP over a new association. +send up to 65535 matching SOP Instances to a known Storage SCP over a new association. This is accomplished through the DIMSE C-MOVE and C-STORE services. One limitation of the C-MOVE service is that the Move SCP/Storage SCU must diff --git a/docs/service_classes/query_retrieve_service_class.rst b/docs/service_classes/query_retrieve_service_class.rst index 7daafe44e..26819d366 100644 --- a/docs/service_classes/query_retrieve_service_class.rst +++ b/docs/service_classes/query_retrieve_service_class.rst @@ -236,6 +236,9 @@ pynetdicom Query/Retrieve (Get) Statuses | 0xC413 | Failure | The handler bound to ``evt.EVT_C_GET`` | | | | yielded an invalid number of sub-operations | +------------------+----------+-----------------------------------------------+ +| 0xC416 | Failure | The handler bound to ``evt.EVT_C_GET`` | +| | | yielded more than 65535 matches | ++------------------+----------+-----------------------------------------------+ .. _qr_move_statuses: @@ -328,3 +331,6 @@ pynetdicom Query/Retrieve (Move) Statuses | 0xC515 | Failure | The handler bound to ``evt.EVT_C_MOVE`` | | | | failed to yield a valid (address, port) pair | +------------------+----------+-----------------------------------------------+ +| 0xC516 | Failure | The handler bound to ``evt.EVT_C_MOVE`` | +| | | yielded more than 65535 matches | ++------------------+----------+-----------------------------------------------+ diff --git a/pynetdicom/service_class.py b/pynetdicom/service_class.py index 015556173..797d9b9f2 100644 --- a/pynetdicom/service_class.py +++ b/pynetdicom/service_class.py @@ -1694,23 +1694,31 @@ def _get_scp(self, req: C_GET, context: "PresentationContext") -> None: "sub-operations value" ) ctx.error_status = 0xC413 - no_suboperations = int(next(generator)) + nr_suboperations = int(next(generator)) if not ctx.success: return - if no_suboperations < 1: + if nr_suboperations < 1: rsp.Status = 0x0000 - rsp.NumberOfRemainingSuboperations = 0 rsp.NumberOfFailedSuboperations = 0 rsp.NumberOfWarningSuboperations = 0 rsp.NumberOfCompletedSuboperations = 0 self.dimse.send_msg(rsp, cx_id) return + if nr_suboperations > 65535: + # Since Number of * Suboperations are US and have a maximum value of 65535, + # and Sections C.4.3.2 and C.4.3.3 require them in the final response, + # that implies no more than 65535 matches + LOGGER.error("The C-GET request handler yielded more than 65535 matches") + rsp.Status = 0xC416 + self.dimse.send_msg(rsp, cx_id) + return + # Track the sub operation results # [remaining, failed, warning, complete] - store_results = [no_suboperations, 0, 0, 0] + store_results = [nr_suboperations, 0, 0, 0] # Store the SOP Instance UIDs from any failed C-STORE sub-operations failed_instances = [] @@ -1797,7 +1805,6 @@ def _add_failed_instance(ds: Dataset) -> None: f"Get SCP Result {ii + 1}: 0x{rsp.Status:04X} " f"({status[0]} - {status[1]})" ) - rsp.NumberOfRemainingSuboperations = None rsp.NumberOfFailedSuboperations = store_results[1] + store_results[0] rsp.NumberOfWarningSuboperations = store_results[2] rsp.NumberOfCompletedSuboperations = store_results[3] @@ -1838,7 +1845,6 @@ def _add_failed_instance(ds: Dataset) -> None: LOGGER.info(f"Get SCP Response {ii + 1}: 0x0000 (Success)") rsp.Identifier = None - rsp.NumberOfRemainingSuboperations = None rsp.NumberOfFailedSuboperations = store_results[1] rsp.NumberOfWarningSuboperations = store_results[2] rsp.NumberOfCompletedSuboperations = store_results[3] @@ -1857,6 +1863,7 @@ def _add_failed_instance(ds: Dataset) -> None: rsp.NumberOfFailedSuboperations = store_results[1] rsp.NumberOfWarningSuboperations = store_results[2] rsp.NumberOfCompletedSuboperations = store_results[3] + self.dimse.send_msg(rsp, cx_id) continue @@ -1961,7 +1968,7 @@ def _add_failed_instance(ds: Dataset) -> None: rsp.Status = 0x0000 rsp.Identifier = None else: - if no_suboperations == store_results[1]: + if nr_suboperations == store_results[1]: # Failure response - all sub-operations failed LOGGER.info(f"Get SCP Response {ii + 2}: 0xA702 (Failure)") rsp.Status = 0xA702 # Unable to perform sub-ops @@ -1982,7 +1989,6 @@ def _add_failed_instance(ds: Dataset) -> None: ) rsp.Identifier = BytesIO(cast(bytes, bytestream)) - rsp.NumberOfRemainingSuboperations = None rsp.NumberOfFailedSuboperations = store_results[1] rsp.NumberOfWarningSuboperations = store_results[2] rsp.NumberOfCompletedSuboperations = store_results[3] @@ -2085,21 +2091,29 @@ def _move_scp(self, req: C_MOVE, context: "PresentationContext") -> None: "sub-operations value" ) ctx.error_status = 0xC513 - no_suboperations = int(next(generator)) + nr_suboperations = int(next(generator)) # Exception in context or handler aborted/released - second yield if not ctx.success or not self.assoc.is_established: return - if no_suboperations < 1: + if nr_suboperations < 1: rsp.Status = 0x0000 - rsp.NumberOfRemainingSuboperations = 0 rsp.NumberOfFailedSuboperations = 0 rsp.NumberOfWarningSuboperations = 0 rsp.NumberOfCompletedSuboperations = 0 self.dimse.send_msg(rsp, cx_id) return + if nr_suboperations > 65535: + # Since Number of * Suboperations are US and have a maximum value of 65535, + # and Sections C.4.2.2 and C.4.2.3 require them in the final response, + # that implies no more than 65535 matches + LOGGER.error("The C-MOVE request handler yielded more than 65535 matches") + rsp.Status = 0xC516 + self.dimse.send_msg(rsp, cx_id) + return + # Try to request new association with Move Destination with attempt(rsp, self.dimse, cx_id) as ctx: ctx.error_msg = ( @@ -2131,7 +2145,7 @@ def _move_scp(self, req: C_MOVE, context: "PresentationContext") -> None: # Track the sub operation results # [remaining, failed, warning, complete] - store_results = [no_suboperations, 0, 0, 0] + store_results = [nr_suboperations, 0, 0, 0] # Store the SOP Instance UIDs from any failed C-STORE sub-operations failed_instances = [] @@ -2243,7 +2257,6 @@ def _add_failed_instance(ds: Dataset) -> None: ) rsp.Identifier = BytesIO(cast(bytes, bytestream)) - rsp.NumberOfRemainingSuboperations = None rsp.NumberOfFailedSuboperations = store_results[1] + store_results[0] rsp.NumberOfWarningSuboperations = store_results[2] rsp.NumberOfCompletedSuboperations = store_results[3] @@ -2275,7 +2288,6 @@ def _add_failed_instance(ds: Dataset) -> None: LOGGER.info(f"Move SCP Response {ii + 1}: 0x0000 (Success)") rsp.Identifier = None - rsp.NumberOfRemainingSuboperations = None rsp.NumberOfFailedSuboperations = store_results[1] rsp.NumberOfWarningSuboperations = store_results[2] rsp.NumberOfCompletedSuboperations = store_results[3] @@ -2367,7 +2379,7 @@ def _add_failed_instance(ds: Dataset) -> None: rsp.Status = 0x0000 rsp.Identifier = None else: - if no_suboperations == store_results[1]: + if nr_suboperations == store_results[1]: # Failure response - all sub-operations failed LOGGER.info(f"Move SCP Response {ii + 2}: 0xA702 (Failure)") rsp.Status = 0xA702 # Unable to perform sub-ops @@ -2388,7 +2400,6 @@ def _add_failed_instance(ds: Dataset) -> None: ) rsp.Identifier = BytesIO(cast(bytes, bytestream)) - rsp.NumberOfRemainingSuboperations = None rsp.NumberOfFailedSuboperations = store_results[1] rsp.NumberOfWarningSuboperations = store_results[2] rsp.NumberOfCompletedSuboperations = store_results[3] diff --git a/pynetdicom/tests/test_service_qr.py b/pynetdicom/tests/test_service_qr.py index fa25da6ae..bbb601e62 100644 --- a/pynetdicom/tests/test_service_qr.py +++ b/pynetdicom/tests/test_service_qr.py @@ -10,6 +10,7 @@ """ from io import BytesIO +import logging import os import time @@ -1390,6 +1391,50 @@ def handle_store(event): assert assoc.is_released scp.shutdown() + def test_get_handler_too_many_subops(self, caplog): + """Test handler yielding more than 65535 subops""" + + def handle(event): + yield 65536 + status = Dataset() + status.Status = 0xFF00 + yield status, self.ds + yield 0x0000, None + + def handle_store(event): + return 0x0000 + + handlers = [(evt.EVT_C_GET, handle), (evt.EVT_C_STORE, handle_store)] + + self.ae = ae = AE() + ae.add_supported_context(PatientRootQueryRetrieveInformationModelGet) + ae.add_supported_context(CTImageStorage, scu_role=False, scp_role=True) + ae.add_requested_context(PatientRootQueryRetrieveInformationModelGet) + ae.add_requested_context(CTImageStorage) + scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers) + + role = build_role(CTImageStorage, scp_role=True) + + ae.acse_timeout = 5 + ae.dimse_timeout = 5 + assoc = ae.associate("localhost", 11112, ext_neg=[role]) + assert assoc.is_established + with caplog.at_level(logging.ERROR, logger="pynetdicom"): + result = assoc.send_c_get( + self.query, PatientRootQueryRetrieveInformationModelGet + ) + status, identifier = next(result) + assert status.Status == 0xC416 + pytest.raises(StopIteration, next, result) + + assoc.release() + assert assoc.is_released + scp.shutdown() + + assert ( + "The C-GET request handler yielded more than 65535 matches" in caplog.text + ) + def test_get_handler_status_dataset(self): """Test handler yielding a Dataset status""" @@ -3869,6 +3914,42 @@ def handle(event): assoc.release() scp.shutdown() + def test_move_handler_too_many_subops(self, caplog): + """Test handler yielding more than 65535 subops""" + + def handle(event): + yield ("127.0.0.1", 11112) + yield 65536 + yield 0xFF00, self.ds + + handlers = [(evt.EVT_C_MOVE, handle)] + + self.ae = ae = AE() + ae.add_supported_context(PatientRootQueryRetrieveInformationModelMove) + ae.add_supported_context(CTImageStorage, scu_role=False, scp_role=True) + ae.add_requested_context(PatientRootQueryRetrieveInformationModelMove) + ae.add_requested_context(CTImageStorage) + scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers) + + ae.acse_timeout = 5 + ae.dimse_timeout = 5 + assoc = ae.associate("localhost", 11112) + assert assoc.is_established + with caplog.at_level(logging.ERROR, logger="pynetdicom"): + result = assoc.send_c_move( + self.query, "TESTMOVE", PatientRootQueryRetrieveInformationModelMove + ) + status, identifier = next(result) + assert status.Status == 0xC516 + pytest.raises(StopIteration, next, result) + + assoc.release() + scp.shutdown() + + assert ( + "The C-MOVE request handler yielded more than 65535 matches" in caplog.text + ) + def test_move_handler_bad_aet(self): """Test handler yielding a bad move aet""" diff --git a/pyproject.toml b/pyproject.toml index d06de01e9..66f032e36 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,6 +17,8 @@ classifiers=[ "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", "Operating System :: OS Independent", "Topic :: Scientific/Engineering :: Medical Science Apps.", "Topic :: Software Development :: Libraries",