From 91cd7ee5aec5498b255a9bdd87793007a562ac7a Mon Sep 17 00:00:00 2001 From: Kieran Brantner-Magee Date: Thu, 12 Nov 2020 09:52:38 -0800 Subject: [PATCH 1/2] Make send_messages, schedule_messages, cancel_scheduled_messages, and receive_deferred_messages gracefully no-op when provided an empty list or empty batch (where appropriate). Adds tests and changelog notes for these scenarios. Closes #15211 --- sdk/servicebus/azure-servicebus/CHANGELOG.md | 2 ++ .../azure/servicebus/_servicebus_receiver.py | 4 ++-- .../azure/servicebus/_servicebus_sender.py | 6 +++++- .../azure/servicebus/aio/_servicebus_receiver_async.py | 4 ++-- .../azure/servicebus/aio/_servicebus_sender_async.py | 6 +++++- .../tests/async_tests/test_queues_async.py | 8 ++++++++ sdk/servicebus/azure-servicebus/tests/test_queues.py | 8 ++++++++ 7 files changed, 32 insertions(+), 6 deletions(-) diff --git a/sdk/servicebus/azure-servicebus/CHANGELOG.md b/sdk/servicebus/azure-servicebus/CHANGELOG.md index 71dd7e187263..1ac5c2c566ea 100644 --- a/sdk/servicebus/azure-servicebus/CHANGELOG.md +++ b/sdk/servicebus/azure-servicebus/CHANGELOG.md @@ -3,7 +3,9 @@ ## 7.0.0b9 (Unreleased) **Breaking Changes** + * `ServiceBusSender` and `ServiceBusReceiver` are no more reusable and will raise `ValueError` when trying to operate on a closed handler. +* `send_messages`, `schedule_messages`, `cancel_scheduled_messages` and `receive_deferred_messages` now performs a no-op rather than raising a `ValueError` if provided an empty list of messages or an empty batch. ## 7.0.0b8 (2020-11-05) diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_receiver.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_receiver.py index ba51b94089aa..797133dd9b6f 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_receiver.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_receiver.py @@ -555,8 +555,8 @@ def receive_deferred_messages(self, sequence_numbers, **kwargs): raise ValueError("The timeout must be greater than 0.") if isinstance(sequence_numbers, six.integer_types): sequence_numbers = [sequence_numbers] - if not sequence_numbers: - raise ValueError("At least one sequence number must be specified.") + if len(sequence_numbers) == 0: + return [] # no-op on empty list. self._open() try: receive_mode = self._receive_mode.value.value diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py index f605fe22a83f..271d42bbc9f4 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py @@ -259,6 +259,8 @@ def schedule_messages(self, messages, schedule_time_utc, **kwargs): if isinstance(messages, ServiceBusMessage): request_body = self._build_schedule_request(schedule_time_utc, messages) else: + if len(messages) == 0: + return [] # No-op on empty list. request_body = self._build_schedule_request(schedule_time_utc, *messages) return self._mgmt_request_response_with_retry( REQUEST_RESPONSE_SCHEDULE_MESSAGE_OPERATION, @@ -296,6 +298,8 @@ def cancel_scheduled_messages(self, sequence_numbers, **kwargs): numbers = [types.AMQPLong(sequence_numbers)] else: numbers = [types.AMQPLong(s) for s in sequence_numbers] + if len(numbers) == 0: + return # no-op on empty list. request_body = {MGMT_REQUEST_SEQUENCE_NUMBERS: types.AMQPArray(numbers)} return self._mgmt_request_response_with_retry( REQUEST_RESPONSE_CANCEL_SCHEDULED_MESSAGE_OPERATION, @@ -347,7 +351,7 @@ def send_messages(self, message, **kwargs): except TypeError: # Message was not a list or generator. pass if isinstance(message, ServiceBusMessageBatch) and len(message) == 0: # pylint: disable=len-as-condition - raise ValueError("A ServiceBusMessageBatch or list of Message must have at least one Message") + return # Short circuit noop if an empty list or batch is provided. if not isinstance(message, ServiceBusMessageBatch) and not isinstance(message, ServiceBusMessage): raise TypeError( "Can only send azure.servicebus. or " diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_receiver_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_receiver_async.py index d09f2801e6dd..30af927836a1 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_receiver_async.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_receiver_async.py @@ -557,8 +557,8 @@ async def receive_deferred_messages( raise ValueError("The timeout must be greater than 0.") if isinstance(sequence_numbers, six.integer_types): sequence_numbers = [sequence_numbers] - if not sequence_numbers: - raise ValueError("At least one sequence number must be specified.") + if len(sequence_numbers) == 0: + return [] # no-op on empty list. await self._open() try: receive_mode = self._receive_mode.value.value diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py index 36178a78663b..40d805266f14 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py @@ -204,6 +204,8 @@ async def schedule_messages( if isinstance(messages, ServiceBusMessage): request_body = self._build_schedule_request(schedule_time_utc, messages) else: + if len(messages) == 0: + return [] # No-op on empty list. request_body = self._build_schedule_request(schedule_time_utc, *messages) return await self._mgmt_request_response_with_retry( REQUEST_RESPONSE_SCHEDULE_MESSAGE_OPERATION, @@ -240,6 +242,8 @@ async def cancel_scheduled_messages(self, sequence_numbers: Union[int, List[int] numbers = [types.AMQPLong(sequence_numbers)] else: numbers = [types.AMQPLong(s) for s in sequence_numbers] + if len(numbers) == 0: + return # no-op on empty list. request_body = {MGMT_REQUEST_SEQUENCE_NUMBERS: types.AMQPArray(numbers)} return await self._mgmt_request_response_with_retry( REQUEST_RESPONSE_CANCEL_SCHEDULED_MESSAGE_OPERATION, @@ -294,7 +298,7 @@ async def send_messages( except TypeError: # Message was not a list or generator. pass if isinstance(message, ServiceBusMessageBatch) and len(message) == 0: # pylint: disable=len-as-condition - raise ValueError("A ServiceBusMessageBatch or list of Message must have at least one Message") + return # Short circuit noop if an empty list or batch is provided. if not isinstance(message, ServiceBusMessageBatch) and not isinstance(message, ServiceBusMessage): raise TypeError( "Can only send azure.servicebus." diff --git a/sdk/servicebus/azure-servicebus/tests/async_tests/test_queues_async.py b/sdk/servicebus/azure-servicebus/tests/async_tests/test_queues_async.py index 478f65a90e92..1c3081d34a3e 100644 --- a/sdk/servicebus/azure-servicebus/tests/async_tests/test_queues_async.py +++ b/sdk/servicebus/azure-servicebus/tests/async_tests/test_queues_async.py @@ -67,6 +67,13 @@ async def test_async_queue_by_queue_client_conn_str_receive_handler_peeklock(sel message = ServiceBusMessage("Handler message no. {}".format(i)) await sender.send_messages(message, timeout=5) + # Test that noop empty send works properly. + await sender.send_messages([]) + await sender.send_messages(ServiceBusMessageBatch()) + assert len(await sender.schedule_messages([], utc_now())) == 0 + await sender.cancel_scheduled_messages([]) + + # Then test expected failure modes. with pytest.raises(ValueError): async with sender: raise AssertionError("Should raise ValueError") @@ -85,6 +92,7 @@ async def test_async_queue_by_queue_client_conn_str_receive_handler_peeklock(sel receiver = sb_client.get_queue_receiver(servicebus_queue.name, max_wait_time=5) async with receiver: + assert len(await receiver.receive_deferred_messages([])) == 0 with pytest.raises(ValueError): await receiver.receive_messages(max_wait_time=0) diff --git a/sdk/servicebus/azure-servicebus/tests/test_queues.py b/sdk/servicebus/azure-servicebus/tests/test_queues.py index 5ec9a3698ac1..b6f303b9a0ff 100644 --- a/sdk/servicebus/azure-servicebus/tests/test_queues.py +++ b/sdk/servicebus/azure-servicebus/tests/test_queues.py @@ -128,8 +128,15 @@ def test_queue_by_queue_client_conn_str_receive_handler_peeklock(self, servicebu message.to = 'to' message.reply_to = 'reply_to' sender.send_messages(message) + + # Test that noop empty send works properly. + sender.send_messages([]) + sender.send_messages(ServiceBusMessageBatch()) + assert len(sender.schedule_messages([], utc_now())) == 0 + sender.cancel_scheduled_messages([]) sender.close() + # Then test expected failure modes. with pytest.raises(ValueError): with sender: raise AssertionError("Should raise ValueError") @@ -145,6 +152,7 @@ def test_queue_by_queue_client_conn_str_receive_handler_peeklock(self, servicebu receiver = sb_client.get_queue_receiver(servicebus_queue.name, max_wait_time=5) + assert len(receiver.receive_deferred_messages([])) == 0 with pytest.raises(ValueError): receiver.receive_messages(max_wait_time=0) From 91954ec384a77d2687e0dc1019af1a4bb656d43e Mon Sep 17 00:00:00 2001 From: Kieran Brantner-Magee Date: Fri, 13 Nov 2020 13:05:26 -0800 Subject: [PATCH 2/2] Change cancel_schedule return noop behavior to align returning None explicitly (lint fix) --- .../azure-servicebus/azure/servicebus/_servicebus_sender.py | 2 +- .../azure/servicebus/aio/_servicebus_sender_async.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py index 271d42bbc9f4..e5c8f4ce9f6c 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_sender.py @@ -299,7 +299,7 @@ def cancel_scheduled_messages(self, sequence_numbers, **kwargs): else: numbers = [types.AMQPLong(s) for s in sequence_numbers] if len(numbers) == 0: - return # no-op on empty list. + return None # no-op on empty list. request_body = {MGMT_REQUEST_SEQUENCE_NUMBERS: types.AMQPArray(numbers)} return self._mgmt_request_response_with_retry( REQUEST_RESPONSE_CANCEL_SCHEDULED_MESSAGE_OPERATION, diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py index 40d805266f14..478120c3d39b 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py @@ -243,7 +243,7 @@ async def cancel_scheduled_messages(self, sequence_numbers: Union[int, List[int] else: numbers = [types.AMQPLong(s) for s in sequence_numbers] if len(numbers) == 0: - return # no-op on empty list. + return None # no-op on empty list. request_body = {MGMT_REQUEST_SEQUENCE_NUMBERS: types.AMQPArray(numbers)} return await self._mgmt_request_response_with_retry( REQUEST_RESPONSE_CANCEL_SCHEDULED_MESSAGE_OPERATION,