Skip to content

Commit

Permalink
[ServiceBus] make amqp_message properties read-only (#14095)
Browse files Browse the repository at this point in the history
In order to allow time for cross-sdk unification on this feature.
  • Loading branch information
KieranBrantnerMagee authored Oct 3, 2020
1 parent 6e5caf2 commit 26f7137
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 63 deletions.
1 change: 1 addition & 0 deletions sdk/servicebus/azure-servicebus/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**Breaking Changes**
* Passing any type other than `ReceiveMode` as parameter `receive_mode` now throws a `TypeError` instead of `AttributeError`.
* Administration Client calls now take only entity names, not `<Entity>Descriptions` as well to reduce ambiguity in which entity was being acted on. TypeError will now be thrown on improper parameter types (non-string).
* `AMQPMessage` (`Message.amqp_message`) properties are now read-only, changes of these properties would not be reflected in the underlying message. This may be subject to change before GA.

## 7.0.0b6 (2020-09-10)

Expand Down
120 changes: 77 additions & 43 deletions sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import uuid
import functools
import logging
import copy
from typing import Optional, List, Union, Iterable, TYPE_CHECKING, Callable, Any

import uamqp.message
Expand Down Expand Up @@ -127,7 +128,7 @@ def __init__(self, body, **kwargs):
self.partition_key = kwargs.pop("partition_key", None)
self.via_partition_key = kwargs.pop("via_partition_key", None)
# If message is the full message, amqp_message is the "public facing interface" for what we expose.
self.amqp_message = AMQPMessage(self.message)
self.amqp_message = AMQPMessage(self.message) # type: AMQPMessage

def __str__(self):
return str(self.message)
Expand Down Expand Up @@ -1083,73 +1084,106 @@ def renew_lock(self):

class AMQPMessage(object):
"""
The internal AMQP message that this ServiceBusMessage represents.
:param properties: Properties to add to the message.
:type properties: ~uamqp.message.MessageProperties
:param application_properties: Service specific application properties.
:type application_properties: dict
:param annotations: Service specific message annotations. Keys in the dictionary
must be `uamqp.types.AMQPSymbol` or `uamqp.types.AMQPuLong`.
:type annotations: dict
:param delivery_annotations: Delivery-specific non-standard properties at the head of the message.
Delivery annotations convey information from the sending peer to the receiving peer.
Keys in the dictionary must be `uamqp.types.AMQPSymbol` or `uamqp.types.AMQPuLong`.
:type delivery_annotations: dict
:param header: The message header.
:type header: ~uamqp.message.MessageHeader
:param footer: The message footer.
:type footer: dict
The internal AMQP message that this ServiceBusMessage represents. Is read-only.
"""
def __init__(self, message):
# type: (uamqp.Message) -> None
self._message = message

@property
def properties(self):
return self._message.properties
# type: () -> uamqp.message.MessageProperties
"""
Properties to add to the message.
@properties.setter
def properties(self, value):
self._message.properties = value
:rtype: ~uamqp.message.MessageProperties
"""
return uamqp.message.MessageProperties(message_id=self._message.properties.message_id,
user_id=self._message.properties.user_id,
to=self._message.properties.to,
subject=self._message.properties.subject,
reply_to=self._message.properties.reply_to,
correlation_id=self._message.properties.correlation_id,
content_type=self._message.properties.content_type,
content_encoding=self._message.properties.content_encoding
)

# NOTE: These are disabled pending arch. design and cross-sdk consensus on
# how we will expose sendability of amqp focused messages. To undo, uncomment and remove deepcopies/workarounds.
#
#@properties.setter
#def properties(self, value):
# self._message.properties = value

@property
def application_properties(self):
return self._message.application_properties
# type: () -> dict
"""
Service specific application properties.
@application_properties.setter
def application_properties(self, value):
self._message.application_properties = value
:rtype: dict
"""
return copy.deepcopy(self._message.application_properties)

#@application_properties.setter
#def application_properties(self, value):
# self._message.application_properties = value

@property
def annotations(self):
return self._message.annotations
# type: () -> dict
"""
Service specific message annotations. Keys in the dictionary
must be `uamqp.types.AMQPSymbol` or `uamqp.types.AMQPuLong`.
:rtype: dict
"""
return copy.deepcopy(self._message.annotations)

@annotations.setter
def annotations(self, value):
self._message.annotations = value
#@annotations.setter
#def annotations(self, value):
# self._message.annotations = value

@property
def delivery_annotations(self):
return self._message.delivery_annotations
# type: () -> dict
"""
Delivery-specific non-standard properties at the head of the message.
Delivery annotations convey information from the sending peer to the receiving peer.
Keys in the dictionary must be `uamqp.types.AMQPSymbol` or `uamqp.types.AMQPuLong`.
@delivery_annotations.setter
def delivery_annotations(self, value):
self._message.delivery_annotations = value
:rtype: dict
"""
return copy.deepcopy(self._message.delivery_annotations)

#@delivery_annotations.setter
#def delivery_annotations(self, value):
# self._message.delivery_annotations = value

@property
def header(self):
return self._message.header
# type: () -> uamqp.message.MessageHeader
"""
The message header.
@header.setter
def header(self, value):
self._message.header = value
:rtype: ~uamqp.message.MessageHeader
"""
return uamqp.message.MessageHeader(header=self._message.header)

#@header.setter
#def header(self, value):
# self._message.header = value

@property
def footer(self):
return self._message.footer
# type: () -> dict
"""
The message footer.
:rtype: dict
"""
return copy.deepcopy(self._message.footer)

@footer.setter
def footer(self, value):
self._message.footer = value
#@footer.setter
#def footer(self, value):
# self._message.footer = value
49 changes: 29 additions & 20 deletions sdk/servicebus/azure-servicebus/tests/test_queues.py
Original file line number Diff line number Diff line change
Expand Up @@ -1873,20 +1873,21 @@ def test_message_inner_amqp_properties(self, servicebus_namespace_connection_str

message = Message("body")

with pytest.raises(TypeError):
with pytest.raises(AttributeError): # Note: If this is made read-writeable, this would be TypeError
message.amqp_message.properties = {"properties":1}
message.amqp_message.properties.subject = "subject"

message.amqp_message.application_properties = {b"application_properties":1}

message.amqp_message.annotations = {b"annotations":2}
message.amqp_message.delivery_annotations = {b"delivery_annotations":3}

with pytest.raises(TypeError):
message.amqp_message.header = {"header":4}
message.amqp_message.header.priority = 5

message.amqp_message.footer = {b"footer":6}
# NOTE: These are disabled pending cross-language-sdk consensus on sendability/writeability.
# message.amqp_message.properties.subject = "subject"
#
# message.amqp_message.application_properties = {b"application_properties":1}
#
# message.amqp_message.annotations = {b"annotations":2}
# message.amqp_message.delivery_annotations = {b"delivery_annotations":3}
#
# with pytest.raises(TypeError):
# message.amqp_message.header = {"header":4}
# message.amqp_message.header.priority = 5
#
# message.amqp_message.footer = {b"footer":6}

with ServiceBusClient.from_connection_string(
servicebus_namespace_connection_string, logging_enable=False) as sb_client:
Expand All @@ -1895,10 +1896,18 @@ def test_message_inner_amqp_properties(self, servicebus_namespace_connection_str
sender.send_messages(message)
with sb_client.get_queue_receiver(servicebus_queue.name, max_wait_time=5) as receiver:
message = receiver.receive_messages()[0]
assert message.amqp_message.properties.subject == b"subject"
assert message.amqp_message.application_properties[b"application_properties"] == 1
assert message.amqp_message.annotations[b"annotations"] == 2
# delivery_annotations and footer disabled pending uamqp bug https://github.com/Azure/azure-uamqp-python/issues/169
#assert message.amqp_message.delivery_annotations[b"delivery_annotations"] == 3
assert message.amqp_message.header.priority == 5
#assert message.amqp_message.footer[b"footer"] == 6
assert message.amqp_message.application_properties == None \
and message.amqp_message.annotations != None \
and message.amqp_message.delivery_annotations != None \
and message.amqp_message.footer == None \
and message.amqp_message.properties != None \
and message.amqp_message.header != None
# NOTE: These are disabled pending cross-language-sdk consensus on sendability/writeability.
#
# assert message.amqp_message.properties.subject == b"subject"
# assert message.amqp_message.application_properties[b"application_properties"] == 1
# assert message.amqp_message.annotations[b"annotations"] == 2
# # delivery_annotations and footer disabled pending uamqp bug https://github.com/Azure/azure-uamqp-python/issues/169
# #assert message.amqp_message.delivery_annotations[b"delivery_annotations"] == 3
# assert message.amqp_message.header.priority == 5
# #assert message.amqp_message.footer[b"footer"] == 6

0 comments on commit 26f7137

Please sign in to comment.