Azure-sdk-for-net: [Discussion] Service Bus - Track 2 - Message Structure

[JoshLove-msft]

_The intent of this issue is to solicit feedback from the community in advance of the GA of the next major version of the Service Bus library (which will follow several Preview releases).
As a reference, the .NET Azure SDK guidelines can be found here: https://azure.github.io/azure-sdk/dotnet_introduction.html
Terminology:
Track 0 refers to the WindowsAzure.ServiceBus library
Track 1 refers to the Microsoft.Azure.ServiceBus library
Track 2 refers to the new library, Azure.Messaging.ServiceBus, we are working on releasing._

In the previous versions of the Service Bus library, there was a single type for both sending and receiving messages. There are a couple downsides to this to go with the obvious upside of its simplicity:

1. System level properties would appear on the message that a user is constructing for sending. Although these properties are read-only, it adds some cognitive overhead to reasoning about the type.
2. When receiving a message, any of the settable properties can still be set. However, setting these properties doesn't actually do anything even after settling the message. For instance a user might think that doing something like the below might actually change the messageId, which it wouldn't. This concern is more pronounced with our proposed addition of overloads for settlement methods taking a message (it could be argued if the settlement only accepts a lock token, then no one would think that changing any of the other properties would affect the settlement).
   ```c#
   var message = await receiver.ReceiveAsync();
   message.MessageId = "newMessageId";
   // in the new version, we are offering overloads for settlement methods
   // that take a message in addition to the ones that take a lock token
   await receiver.AbandonAsync(message);

In order to address these two issues, we are proposing having two separate types for messages:
- A ServiceBusMessage which is what a user would create for sending. 
- A ServiceBusReceivedMessage which is what a user would get back when receiving a message.

Intially, we thought that ServiceBusReceivedMessage could derive from ServiceBusMessage, but the issue we ran into there was needing to somehow still make the received message properties read-only (we could do runtime validation or use "new" keyword), in addition to wanting to make it super clear to users that re-sending a message that has system properties set on it, will not end up having those same system properties set once it is processed by the service (these system properties would be thrown out before sending).

So our updated proposal is to not have ServiceBusReceivedMessage derive from ServiceBusMessage. This allows us to keep ServiceBusReceivedMessage immutable. It also means we don't have to worry about a user sending a received message. In order to support re-sending a received message we offer a static factory method.
```c#
foreach (ServiceBusReceivedMessage msg in receivedMessages)
{
    ServiceBusMessage sendableMessage = ServiceBusMessage.CreateFrom(receivedMessage);
    await sender.SendAsync(sendableMessage);
}

The downside to breaking the hierarchy is that code that could be written to process a message in some way that should be agnostic to whether the message was sent or received, would now have to be duplicated somewhat. We would like to see what the community thinks about this tradeoff.

[SeanFeldman]

What are the benefits of

ServiceBusMessage sendableMessage = ServiceBusMessage.CreateFrom(receivedMessage);

over

ServiceBusMessage sendableMessage = new ServiceBusMessage(receivedMessage);

[SeanFeldman]

The downside to breaking the hierarchy is that code that could be written to process a message in some way that should be agnostic to whether the message was sent or received, would now have to be duplicated somewhat. We would like to see what the community thinks about this tradeoff.

Reuse but don't abuse. In most scenarios I've encountered, the direction of the message matters. It always has to be a ServiceBusReceivedMessage to know till when it's locked. And it always has to be a ServiceBusMessage to set Enqueued time. The split makes sense.

[JoshLove-msft]

What are the benefits of

ServiceBusMessage sendableMessage = ServiceBusMessage.CreateFrom(receivedMessage);

over

ServiceBusMessage sendableMessage = new ServiceBusMessage(receivedMessage);

Mainly that we don't expect this to be a super common scenario. That was the thinking behind using the static factory method vs. a constructor.

[SeanFeldman]

If I'm given a choice of how to construct a new message, using a constructor vs a factory method, a constructor if the first thing on my mind. Not a factory type or method. And if discoverability matters, "dotting into CreateFrom" has a higher mental complexity than instantiating a new SB message and seeing the right overload. Not to mention that _any_ ASB developer is going to create many messages, familiarizing themselves with the construction options fairly quickly.

[JoshLove-msft]

If I'm given a choice of how to construct a new message, using a constructor vs a factory method, a constructor if the first thing on my mind. Not a factory type or method. And if discoverability matters, "dotting into CreateFrom" has a higher mental complexity than instantiating a new SB message and seeing the right overload. Not to mention that _any_ ASB developer is going to create many messages, familiarizing themselves with the construction options fairly quickly.

We were pretty on the fence on this one. Thanks for the feedback.

[JoshLove-msft]

Adding some community members who have recently submitted issues/PRs for Service Bus 😃

/cc @nemakam @SeanFeldman @Einarsson @brandondahler @broloaugh @ankrause @GrimGadget @ShaneCourtrille @Pucmaster @Bnjmn83

[KrzysztofCwalina]

I am not totally opposed to the ctor, but I think many .NET developers think that such ctors would wrap the input parameter. The Create method makes it clearer that a data is copied.

[SeanFeldman]

I am not totally opposed to the ctor, but I think many .NET developers think that such ctors would wrap the input parameter. The Create method makes it clearer that a data is copied.

That's why there's XML documentation to elaborate on the methods, parameters, and provide remarks if needed. Which will also exist on the factory method.

You either educate extremely quickly that a new ServiceBusMessage can be created by passing a source message into the constructor or you let developers discover it by dotting to the method. From what I've seen, .Clone() was not very discoverable and trivial even for those that have had some SDK work.

[ShivangiReja]

/cc @danielmarbach

[danielmarbach]

This is a difficult topic because it seems it comes down to a matter of almost "taste".

I am not totally opposed to the ctor, but I think many .NET developers think that such ctors would wrap the input parameter. The Create method makes it clearer that a data is copied.

I tend to agree with that but also don't have strong objections for what Sean proposed.

Just to make things worse I'm going to throw in a third approach without implying any preference.

It would also be possible to turn things around and have an extension method on ServiceBusReceivedMessage that allows to do something like

var messageToSend = receivedMessage.To|ToSendMessage|PickAnyNameYoulikeAndDontGetHungup();

as long as the extension method is in the same namespace it is immediately discoverable when doing receivedMessage.<magic of intellisense here>

[danielmarbach]

Unfortunately @KrzysztofCwalina post is no longer available but somehow copied the gist of it into a stack overflow post

The most common and consistent way to create an instance of a type is via its constructor. However, sometimes a preferable alternative is to use the Factory pattern.

Do prefer constructors over Factories as they are generally more consistent and convenient than specialized construction mechanisms.

Do implement Factory operations as methods, not properties.

Do return instances as method return values, not as out parameters.

Consider using a Factory if you need more control over the creation patterns of instances.

Consider naming Factory methods by concatenating “Create” and the name of the type being created.

Consider naming Factory types by concatenating the name of type being created and “Factory.”

Do not implement your Factory using a static method if the construction operation must be available for subclasses to specialize.

Do use a Factory for conversion style operations.

Do use a Factory if an operation requires parameter information which feels unnatural to pass to a constructor.

Do not use a Factory in situations where the creation operation will be used as a core scenario for instantiation of a type. In these situations, discoverability and consistency are paramount.

Consider using a constructor instead of a Factory. Only after this thought process should you proceed with the implementation of a Factory.

Factories are also often handy to enable type specialization through polymorphism.

Do use a Factory if API users will be coding to a base class or interface whose implementation is subject to change over time.

Do use Factory methods in cases where a developer might not know which type to construct, for instance when coding against a base type or interface.

Do implement Factory operations as virtual instance methods rather than static if they must support polymorphic extension.

Do use the Singleton pattern for Factories which are instance based so as not to force developers to instantiate a Factory type just to invoke one of its members.

Do use a Factory method if a constructor would be insufficient to describe the operation being performed, and the additional information gained from an individually named Factory makes an operation’s purpose clearer.

https://stackoverflow.com/questions/2959871/factory-vs-instance-constructors

Based on the above and the principle of evolvability I'm leaning more towards the method based than the constructor based approach. It also allows more elaborate manipulation of the data should that ever be needed and conveys better the message of "this might be a bit more expensive than simply creating a new object"

[JoshLove-msft]

This is a difficult topic because it seems it comes down to a matter of almost "taste".

I am not totally opposed to the ctor, but I think many .NET developers think that such ctors would wrap the input parameter. The Create method makes it clearer that a data is copied.

I tend to agree with that but also don't have strong objections for what Sean proposed.

Just to make things worse I'm going to throw in a third approach without implying any preference.

It would also be possible to turn things around and have an extension method on ServiceBusReceivedMessage that allows to do something like

var messageToSend = receivedMessage.To|ToSendMessage|PickAnyNameYoulikeAndDontGetHungup();

as long as the extension method is in the same namespace it is immediately discoverable when doing receivedMessage.<magic of intellisense here>

I do think this would be more discoverable than the static factory method on ServiceBusMessage. I think the main downside is that it isn't clear that you aren't mutating the receivedMessage itself.

[JoshLove-msft]

Based on data we have gathered in some UX studies, I'm leaning towards a constructor overload for creating a new message from a received message. Participants didn't have much success in finding the static factory method. They generally looked for a ctor overload or an instance method on the ServiceBusReceivedMessage.

[SeanFeldman]

I can retire now 😂
Thanks for confirming @JoshLove-msft.