[ServiceBus] add keyword override support to update_ methods in mgmt …

…module (#18210) * add keyword override support * updat impl * add test and recordings * version bump, changelog update * bug fix log * fix pylint * fix pylint and mypy * pr review feedback * Update sdk/servicebus/azure-servicebus/CHANGELOG.md Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com> * review feedback * fix mypy and pylint * fix pylint * header bearer should use normalized path to generate token instead of raw input * update code snippets to show update keyword argument usage Co-authored-by: swathipil <76007337+swathipil@users.noreply.github.com>
Azure · May 5, 2021 · 17c2f18 · 17c2f18
1 parent 701a6f5
commit 17c2f18
Show file tree

Hide file tree

Showing 36 changed files with 2,984 additions and 1,312 deletions.
diff --git a/sdk/servicebus/azure-servicebus/CHANGELOG.md b/sdk/servicebus/azure-servicebus/CHANGELOG.md
@@ -8,6 +8,11 @@ The preview features related to AMQPAnnotatedMessage introduced in 7.2.0b1 are n
 
 * Added support for using `azure.core.credentials.AzureNamedKeyCredential` as credential for authenticating the clients.
 * Support for using `azure.core.credentials.AzureSasCredential` as credential for authenticating the clients is now GA.
+* `ServiceBusAdministrationClient.update_*` methods now accept keyword arguments to override the properties specified in the model instance.
+
+**Bug Fixes**
+
+* Fixed a bug where `update_queue` and `update_subscription` methods were mutating the properties `forward_to` and `forward_dead_lettered_messages_to` of the model instance when those properties are entities instead of full paths.
 
 **Notes**
 
@@ -26,6 +31,7 @@ The preview features related to AMQPAnnotatedMessage introduced in 7.2.0b1 are n
   - `VALUE`: The body of message consists of one amqp-value section and the section contains a single AMQP value.
 * Added new property `body_type` on `azure.servicebus.ServiceBusMessage` and `azure.servicebus.ReceivedMessage` which returns `azure.servicebus.AMQPMessageBodyType`.
 
+
 ## 7.1.1 (2021-04-07)
 
 This version and all future versions will require Python 2.7 or Python 3.6+, Python 3.5 is no longer supported.

diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/management/_management_client_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/management/_management_client_async.py
@@ -6,6 +6,7 @@
 # pylint:disable=specify-parameter-names-in-call
 # pylint:disable=too-many-lines
 import functools
+from copy import deepcopy
 from typing import TYPE_CHECKING, Any, Union, cast, Mapping
 from xml.etree.ElementTree import ElementTree
 
@@ -72,7 +73,6 @@
 )
 from ...management._xml_workaround_policy import ServiceBusXMLWorkaroundPolicy
 from ...management._handle_response_error import _handle_response_error
-from ...management._model_workaround import avoid_timedelta_overflow
 from ._utils import extract_data_template, extract_rule_data_template, get_next_template
 from ...management._utils import (
     deserialize_rule_key_values,
@@ -386,14 +386,14 @@ async def create_queue(self, queue_name: str, **kwargs) -> QueueProperties:
             forward_dead_lettered_messages_to=forward_dead_lettered_messages_to,
             user_metadata=kwargs.pop("user_metadata", None),
         )
-        to_create = queue._to_internal_entity()
+        to_create = queue._to_internal_entity(self.fully_qualified_namespace)
         create_entity_body = CreateQueueBody(
             content=CreateQueueBodyContent(
                 queue_description=to_create,  # type: ignore
             )
         )
         request_body = create_entity_body.serialize(is_xml=True)
-        await self._create_forward_to_header_tokens(queue, kwargs)
+        await self._create_forward_to_header_tokens(to_create, kwargs)
         with _handle_response_error():
             entry_ele = cast(
                 ElementTree,
@@ -419,39 +419,26 @@ async def update_queue(
         Before calling this method, you should use `get_queue`, `create_queue` or `list_queues` to get a
         `QueueProperties` instance, then update the properties. Only a portion of properties can
         be updated. Refer to https://docs.microsoft.com/en-us/rest/api/servicebus/update-queue.
+        You could also pass keyword arguments for updating properties in the form of
+        `<property_name>=<property_value>` which will override whatever was specified in
+        the `QueueProperties` instance. Refer to ~azure.servicebus.management.QueueProperties for names of properties.
 
         :param queue: The queue that is returned from `get_queue`, `create_queue` or `list_queues` and
          has the updated properties.
         :type queue: ~azure.servicebus.management.QueueProperties
         :rtype: None
         """
-
-        queue = create_properties_from_dict_if_needed(queue, QueueProperties)
-        queue.forward_to = _normalize_entity_path_to_full_path_if_needed(
-            queue.forward_to, self.fully_qualified_namespace
-        )
-        queue.forward_dead_lettered_messages_to = (
-            _normalize_entity_path_to_full_path_if_needed(
-                queue.forward_dead_lettered_messages_to,
-                self.fully_qualified_namespace,
-            )
-        )
-        to_update = queue._to_internal_entity()
-
-        to_update.default_message_time_to_live = avoid_timedelta_overflow(
-            to_update.default_message_time_to_live
-        )
-        to_update.auto_delete_on_idle = avoid_timedelta_overflow(
-            to_update.auto_delete_on_idle
-        )
+        # we should not mutate the input, making a copy first for update
+        queue = deepcopy(create_properties_from_dict_if_needed(queue, QueueProperties))
+        to_update = queue._to_internal_entity(self.fully_qualified_namespace, kwargs)
 
         create_entity_body = CreateQueueBody(
             content=CreateQueueBodyContent(
                 queue_description=to_update,
             )
         )
         request_body = create_entity_body.serialize(is_xml=True)
-        await self._create_forward_to_header_tokens(queue, kwargs)
+        await self._create_forward_to_header_tokens(to_update, kwargs)
         with _handle_response_error():
             await self._impl.entity.put(
                 queue.name,  # type: ignore
@@ -660,22 +647,18 @@ async def update_topic(
         Before calling this method, you should use `get_topic`, `create_topic` or `list_topics` to get a
         `TopicProperties` instance, then update the properties. Only a portion of properties can be updated.
         Refer to https://docs.microsoft.com/en-us/rest/api/servicebus/update-topic.
+        You could also pass keyword arguments for updating properties in the form of
+        `<property_name>=<property_value>` which will override whatever was specified in
+        the `TopicProperties` instance. Refer to ~azure.servicebus.management.TopicProperties for names of properties.
 
         :param topic: The topic that is returned from `get_topic`, `create_topic`, or `list_topics`
          and has the updated properties.
         :type topic: ~azure.servicebus.management.TopicProperties
         :rtype: None
         """
 
-        topic = create_properties_from_dict_if_needed(topic, TopicProperties)
-        to_update = topic._to_internal_entity()
-
-        to_update.default_message_time_to_live = avoid_timedelta_overflow(  # type: ignore
-            to_update.default_message_time_to_live
-        )
-        to_update.auto_delete_on_idle = avoid_timedelta_overflow(  # type: ignore
-            to_update.auto_delete_on_idle
-        )
+        topic = deepcopy(create_properties_from_dict_if_needed(topic, TopicProperties))
+        to_update = topic._to_internal_entity(kwargs)
 
         create_entity_body = CreateTopicBody(
             content=CreateTopicBodyContent(
@@ -849,6 +832,7 @@ async def create_subscription(
         :type auto_delete_on_idle: Union[~datetime.timedelta, str]
         :rtype:  ~azure.servicebus.management.SubscriptionProperties
         """
+        # pylint:disable=protected-access
         _validate_entity_name_type(topic_name, display_name="topic_name")
         forward_to = _normalize_entity_path_to_full_path_if_needed(
             kwargs.pop("forward_to", None), self.fully_qualified_namespace
@@ -882,15 +866,15 @@ async def create_subscription(
             auto_delete_on_idle=kwargs.pop("auto_delete_on_idle", None),
             availability_status=None,
         )
-        to_create = subscription._to_internal_entity()  # type: ignore  # pylint:disable=protected-access
+        to_create = subscription._to_internal_entity(self.fully_qualified_namespace)  # type: ignore
 
         create_entity_body = CreateSubscriptionBody(
             content=CreateSubscriptionBodyContent(
                 subscription_description=to_create,  # type: ignore
             )
         )
         request_body = create_entity_body.serialize(is_xml=True)
-        await self._create_forward_to_header_tokens(subscription, kwargs)
+        await self._create_forward_to_header_tokens(to_create, kwargs)
         with _handle_response_error():
             entry_ele = cast(
                 ElementTree,
@@ -919,6 +903,10 @@ async def update_subscription(
 
         Before calling this method, you should use `get_subscription`, `update_subscription` or `list_subscription`
         to get a `SubscriptionProperties` instance, then update the properties.
+        You could also pass keyword arguments for updating properties in the form of
+        `<property_name>=<property_value>` which will override whatever was specified in
+        the `SubscriptionProperties` instance.
+        Refer to ~azure.servicebus.management.SubscriptionProperties for names of properties.
 
         :param str topic_name: The topic that owns the subscription.
         :param ~azure.servicebus.management.SubscriptionProperties subscription: The subscription that is returned
@@ -927,35 +915,17 @@ async def update_subscription(
         """
 
         _validate_entity_name_type(topic_name, display_name="topic_name")
-
-        subscription = create_properties_from_dict_if_needed(
-            subscription, SubscriptionProperties
-        )
-        subscription.forward_to = _normalize_entity_path_to_full_path_if_needed(
-            subscription.forward_to, self.fully_qualified_namespace
-        )
-        subscription.forward_dead_lettered_messages_to = (
-            _normalize_entity_path_to_full_path_if_needed(
-                subscription.forward_dead_lettered_messages_to,
-                self.fully_qualified_namespace,
-            )
-        )
-        to_update = subscription._to_internal_entity()
-
-        to_update.default_message_time_to_live = avoid_timedelta_overflow(  # type: ignore
-            to_update.default_message_time_to_live
-        )
-        to_update.auto_delete_on_idle = avoid_timedelta_overflow(  # type: ignore
-            to_update.auto_delete_on_idle
-        )
+        # we should not mutate the input, making a copy first for update
+        subscription = deepcopy(create_properties_from_dict_if_needed(subscription, SubscriptionProperties))
+        to_update = subscription._to_internal_entity(self.fully_qualified_namespace, kwargs)
 
         create_entity_body = CreateSubscriptionBody(
             content=CreateSubscriptionBodyContent(
                 subscription_description=to_update,
             )
         )
         request_body = create_entity_body.serialize(is_xml=True)
-        await self._create_forward_to_header_tokens(subscription, kwargs)
+        await self._create_forward_to_header_tokens(to_update, kwargs)
         with _handle_response_error():
             await self._impl.subscription.put(
                 topic_name,
@@ -1130,6 +1100,9 @@ async def update_rule(
 
         Before calling this method, you should use `get_rule`, `create_rule` or `list_rules` to get a `RuleProperties`
         instance, then update the properties.
+        You could also pass keyword arguments for updating properties in the form of
+        `<property_name>=<property_value>` which will override whatever was specified in
+        the `RuleProperties` instance. Refer to ~azure.servicebus.management.RuleProperties for names of properties.
 
         :param str topic_name: The topic that owns the subscription.
         :param str subscription_name: The subscription that
@@ -1140,9 +1113,9 @@ async def update_rule(
         :rtype: None
         """
         _validate_topic_and_subscription_types(topic_name, subscription_name)
-
-        rule = create_properties_from_dict_if_needed(rule, RuleProperties)
-        to_update = rule._to_internal_entity()
+        # we should not mutate the input, making a copy first for update
+        rule = deepcopy(create_properties_from_dict_if_needed(rule, RuleProperties))
+        to_update = rule._to_internal_entity(kwargs)
 
         create_entity_body = CreateRuleBody(
             content=CreateRuleBodyContent(