From 65b9cb536a2d9922a7e2f5d9ac721b423f4a2661 Mon Sep 17 00:00:00 2001 From: Kieran Brantner-Magee Date: Tue, 10 Nov 2020 11:05:04 -0800 Subject: [PATCH 1/2] Enable FQDNs and connection strings to support newlines and protocol prefixing (e.g. sb://) and add tests to this effect. --- sdk/servicebus/azure-servicebus/CHANGELOG.md | 3 ++ .../azure/servicebus/_base_handler.py | 13 +++--- .../azure/servicebus/_common/utils.py | 9 ++++ .../azure/servicebus/_servicebus_client.py | 6 ++- .../servicebus/aio/_base_handler_async.py | 5 ++- .../aio/_servicebus_client_async.py | 5 ++- .../tests/stress_tests/stress_test_base.py | 5 +-- .../azure-servicebus/tests/test_queues.py | 3 ++ .../azure-servicebus/tests/test_sb_client.py | 41 ++++++++++++++++++- 9 files changed, 72 insertions(+), 18 deletions(-) diff --git a/sdk/servicebus/azure-servicebus/CHANGELOG.md b/sdk/servicebus/azure-servicebus/CHANGELOG.md index 50cdf9aa6b07..95f958db7f06 100644 --- a/sdk/servicebus/azure-servicebus/CHANGELOG.md +++ b/sdk/servicebus/azure-servicebus/CHANGELOG.md @@ -2,6 +2,9 @@ ## 7.0.0b9 (Unreleased) +**BugFixes** + +* FQDNs and Connection strings are now supported even with strippable whitespace or protocol headers (e.g. 'sb://') ## 7.0.0b8 (2020-11-05) diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_base_handler.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_base_handler.py index dc84504b29db..8f914c17b6f2 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/_base_handler.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/_base_handler.py @@ -26,7 +26,7 @@ OperationTimeoutError, _create_servicebus_exception ) -from ._common.utils import create_properties +from ._common.utils import create_properties, strip_protocol_from_uri from ._common.constants import ( CONTAINER_PREFIX, MANAGEMENT_PATH_SUFFIX, @@ -49,7 +49,7 @@ def _parse_conn_str(conn_str): entity_path = None # type: Optional[str] shared_access_signature = None # type: Optional[str] shared_access_signature_expiry = None # type: Optional[int] - for element in conn_str.split(";"): + for element in conn_str.strip().split(";"): key, _, value = element.partition("=") if key.lower() == "endpoint": endpoint = value.rstrip("/") @@ -78,11 +78,7 @@ def _parse_conn_str(conn_str): "\nWith alternate option of providing SharedAccessSignature instead of SharedAccessKeyName and Key" ) entity = cast(str, entity_path) - left_slash_pos = cast(str, endpoint).find("//") - if left_slash_pos != -1: - host = cast(str, endpoint)[left_slash_pos + 2:] - else: - host = str(endpoint) + host = cast(str, strip_protocol_from_uri(cast(str, endpoint))) return (host, str(shared_access_key_name) if shared_access_key_name else None, @@ -162,7 +158,8 @@ def __init__( **kwargs ): # type: (str, str, TokenCredential, Any) -> None - self.fully_qualified_namespace = fully_qualified_namespace + # If the user provided http:// or sb://, let's be polite and strip that. + self.fully_qualified_namespace = strip_protocol_from_uri(fully_qualified_namespace.strip()) self._entity_name = entity_name subscription_name = kwargs.get("subscription_name") diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py index 1a8f38d7cdfa..bf005b3aaf49 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py @@ -174,3 +174,12 @@ def transform_messages_to_sendable_if_needed(messages): return messages._to_outgoing_message() except AttributeError: return messages + + +def strip_protocol_from_uri(uri): + # type: (str) -> str + """Removes the protocol (e.g. http:// or sb://) from a URI, such as the FQDN.""" + left_slash_pos = uri.find("//") + if left_slash_pos != -1: + return uri[left_slash_pos + 2:] + return uri diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_client.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_client.py index cbf23d6bffb7..8b9c7563c07d 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_client.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_client.py @@ -15,7 +15,7 @@ from ._servicebus_sender import ServiceBusSender from ._servicebus_receiver import ServiceBusReceiver from ._common._configuration import Configuration -from ._common.utils import create_authentication, generate_dead_letter_entity_name +from ._common.utils import create_authentication, generate_dead_letter_entity_name, strip_protocol_from_uri from ._common.constants import SubQueue if TYPE_CHECKING: @@ -70,7 +70,9 @@ def __init__( **kwargs ): # type: (str, TokenCredential, Any) -> None - self.fully_qualified_namespace = fully_qualified_namespace + # If the user provided http:// or sb://, let's be polite and strip that. + self.fully_qualified_namespace = strip_protocol_from_uri(fully_qualified_namespace.strip()) + self._credential = credential self._config = Configuration(**kwargs) self._connection = None diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py index c16e9518c148..08d01ec78417 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py @@ -16,7 +16,7 @@ from .._base_handler import _generate_sas_token, BaseHandler as BaseHandlerSync from .._common._configuration import Configuration -from .._common.utils import create_properties +from .._common.utils import create_properties, strip_protocol_from_uri from .._common.constants import ( TOKEN_TYPE_SASTOKEN, MGMT_REQUEST_OP_TYPE_ENTITY_MGMT, @@ -81,7 +81,8 @@ def __init__( credential: "TokenCredential", **kwargs: Any ) -> None: - self.fully_qualified_namespace = fully_qualified_namespace + # If the user provided http:// or sb://, let's be polite and strip that. + self.fully_qualified_namespace = strip_protocol_from_uri(fully_qualified_namespace.strip()) self._entity_name = entity_name subscription_name = kwargs.get("subscription_name") diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_client_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_client_async.py index 78b05235dd4c..27d8b1a2e1cf 100644 --- a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_client_async.py +++ b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_client_async.py @@ -12,7 +12,7 @@ from ._servicebus_sender_async import ServiceBusSender from ._servicebus_receiver_async import ServiceBusReceiver from .._common._configuration import Configuration -from .._common.utils import generate_dead_letter_entity_name +from .._common.utils import generate_dead_letter_entity_name, strip_protocol_from_uri from .._common.constants import SubQueue from ._async_utils import create_authentication @@ -66,7 +66,8 @@ def __init__( credential: "TokenCredential", **kwargs: Any ) -> None: - self.fully_qualified_namespace = fully_qualified_namespace + # If the user provided http:// or sb://, let's be polite and strip that. + self.fully_qualified_namespace = strip_protocol_from_uri(fully_qualified_namespace.strip()) self._credential = credential self._config = Configuration(**kwargs) self._connection = None diff --git a/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py b/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py index 7d276ea53796..8e6002896b96 100644 --- a/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py +++ b/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py @@ -81,7 +81,7 @@ def __init__(self, should_complete_messages = True, max_message_count = 1, send_session_id = None, - fail_on_exception = True): + fail_on_exception = False): self.senders = senders self.receivers = receivers self.duration=duration @@ -149,8 +149,7 @@ def _schedule_interval_logger(self, end_time, description="", interval_seconds=3 def _do_interval_logging(): if end_time > datetime.utcnow() and not self._should_stop: self._state.populate_process_stats() - _logger.critical("{} RECURRENT STATUS:".format(description)) - _logger.critical(self._state) + _logger.critical("{} RECURRENT STATUS: {}".format(description, self._state)) self._schedule_interval_logger(end_time, description, interval_seconds) t = threading.Timer(interval_seconds, _do_interval_logging) diff --git a/sdk/servicebus/azure-servicebus/tests/test_queues.py b/sdk/servicebus/azure-servicebus/tests/test_queues.py index 1b8e2c2db042..2fa2a100fd70 100644 --- a/sdk/servicebus/azure-servicebus/tests/test_queues.py +++ b/sdk/servicebus/azure-servicebus/tests/test_queues.py @@ -1093,6 +1093,9 @@ def test_queue_message_connection_closed(self, servicebus_namespace_connection_s with pytest.raises(MessageSettleFailed): receiver.complete_message(messages[0]) + + with pytest.raises(MessageSettleFailed): + receiver.receive_messages(max_wait_time=1) @pytest.mark.liveTest diff --git a/sdk/servicebus/azure-servicebus/tests/test_sb_client.py b/sdk/servicebus/azure-servicebus/tests/test_sb_client.py index 1acb94af9d88..b59a7c89f6a3 100644 --- a/sdk/servicebus/azure-servicebus/tests/test_sb_client.py +++ b/sdk/servicebus/azure-servicebus/tests/test_sb_client.py @@ -224,4 +224,43 @@ def test_client_sas_credential(self, #with client: # assert len(client._handlers) == 0 # with client.get_queue_sender(servicebus_queue.name) as sender: - # sender.send_messages(ServiceBusMessage("foo")) \ No newline at end of file + # sender.send_messages(ServiceBusMessage("foo")) + + @pytest.mark.liveTest + @pytest.mark.live_test_only + @CachedResourceGroupPreparer() + @CachedServiceBusNamespacePreparer(name_prefix='servicebustest') + @CachedServiceBusQueuePreparer(name_prefix='servicebustest') + def test_client_credential(self, + servicebus_queue, + servicebus_namespace, + servicebus_namespace_key_name, + servicebus_namespace_primary_key, + servicebus_namespace_connection_string, + **kwargs): + # This should "just work" to validate known-good. + credential = ServiceBusSharedKeyCredential(servicebus_namespace_key_name, servicebus_namespace_primary_key) + hostname = "{}.servicebus.windows.net".format(servicebus_namespace.name) + + client = ServiceBusClient(hostname, credential) + with client: + assert len(client._handlers) == 0 + with client.get_queue_sender(servicebus_queue.name) as sender: + sender.send_messages(ServiceBusMessage("foo")) + + hostname = "sb://{}.servicebus.windows.net".format(servicebus_namespace.name) + + client = ServiceBusClient(hostname, credential) + with client: + assert len(client._handlers) == 0 + with client.get_queue_sender(servicebus_queue.name) as sender: + sender.send_messages(ServiceBusMessage("foo")) + + hostname = "https://{}.servicebus.windows.net \ + ".format(servicebus_namespace.name) + + client = ServiceBusClient(hostname, credential) + with client: + assert len(client._handlers) == 0 + with client.get_queue_sender(servicebus_queue.name) as sender: + sender.send_messages(ServiceBusMessage("foo")) From 5018aa13b5b2c4e3bccb1977c2ff5b15f02c7e21 Mon Sep 17 00:00:00 2001 From: Kieran Brantner-Magee Date: Tue, 10 Nov 2020 11:14:18 -0800 Subject: [PATCH 2/2] revert some test changes that slipped in (will be needed when we remove lazy reopen) --- .../azure-servicebus/tests/stress_tests/stress_test_base.py | 2 +- sdk/servicebus/azure-servicebus/tests/test_queues.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py b/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py index 8e6002896b96..f9782cc6a894 100644 --- a/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py +++ b/sdk/servicebus/azure-servicebus/tests/stress_tests/stress_test_base.py @@ -81,7 +81,7 @@ def __init__(self, should_complete_messages = True, max_message_count = 1, send_session_id = None, - fail_on_exception = False): + fail_on_exception = True): self.senders = senders self.receivers = receivers self.duration=duration diff --git a/sdk/servicebus/azure-servicebus/tests/test_queues.py b/sdk/servicebus/azure-servicebus/tests/test_queues.py index 2fa2a100fd70..1b8e2c2db042 100644 --- a/sdk/servicebus/azure-servicebus/tests/test_queues.py +++ b/sdk/servicebus/azure-servicebus/tests/test_queues.py @@ -1093,9 +1093,6 @@ def test_queue_message_connection_closed(self, servicebus_namespace_connection_s with pytest.raises(MessageSettleFailed): receiver.complete_message(messages[0]) - - with pytest.raises(MessageSettleFailed): - receiver.receive_messages(max_wait_time=1) @pytest.mark.liveTest