Azure-sdk-for-net: [Discussion] Service Bus - Track 2 - Client Hierarchy

[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._

This issue specifically covers the client hierarchy we are proposing.

Guiding principles

• Approachability
  The perceived complexity of an API increases as the number of types increase, particularly when it comes to types that users need to construct directly.

• Consistency
  Have a consistent API across use cases, particularly when comparing session vs non-session use case.
• Resource usage
  Connections are expensive in the AMQP world and should be reused where possible. Connection sharing is critical for session use cases where a client is expected to handle many sessions, and a separate receiving link is needed for each session.

Based on these principles, we arrived at the idea of a having a single top level client, called ServiceBusClient, that users would construct in order to use Service Bus.

Approachability
The ServiceBusClient helps with the discoverability of the API as you can dot into the client and see all the available methods as opposed to searching through documentation or exploring namespace for the types that you can instantiate. Whether sending or receiving, or using sessions or not, users will start their applications by constructing the same client.

Consistency
This client also presents an opportunity for making session/non-session usage a bit more consistent as compared to previous libraries. In the previous library, there was a different top-level client when working with sessions, called SessionClient. This client could be used to spawn off receiver instances by calling AcceptMessageSessionAsync.

In Track 2, we are making an attempt to have the session/non-session usage be as seamless as possible. This allows users to make less changes to their code if they decide they want to start or stop using sessions in an already coded app.

Resource usage
By using a single top-level client, we can implicitly share a single connection for all operations that an application performs. In the previous library, connection sharing was implicit when using the SessionClient, but when using other clients, users would need to explicitly pass in a ServiceBusConnection object in order to share a connection. By making this sharing be implicit to a ServiceBusClient instance (in fact ServiceBusConnection will no longer be exposed), we can help ensure that applications will not use multiple connections unless they explicitly opt in by creating multiple ServiceBusClient instances.

Creating a ServiceBusClient
The ServiceBusClient can be created using either TokenCredential or connection string.
```c#
// connectionString as generated from the Portal
var client = new ServiceBusClient(connectionString);

// fullyQualifiedNamespace would be the account namespace,
// e.g. mynamespace.servicebus.windows.net
// tokenCredential would be a credential of type Azure.Core.TokenCredential
var client = new ServiceBusClient(fullyQualifiedNamespace, tokenCredential);

**Sending and Receiving**
In the typical use case, an app will either be sending or receiving from a particular entity. As such, it makes sense to split the API into sending/receiving. 

```c#
// In order to create a sender
ServiceBusSender sender  = client.CreateSender("myQueueOrTopic");

// Similarly for creating a receiver, you would call 
ServiceBusReceiver receiver = client.CreateReceiver("myQueue");

// or for subscriptions
ServiceBusReceiver receiver = client.CreateReceiver("myTopic", "mySubscription");

Receiving from sessions
When working with sessions, users can receive from a particular session by providing a session id or they can choose to receive from any available session. As such we have a different type for session receiving and different methods for creating a session receiver.

```c#
// In order to create a receiver scoped to the next available session
ServiceBusSessionReceiver receiver = await client.CreateSessionReceiverAsync("myqueue");

// In order to create a receiver scoped to a specific session
ServiceBusSessionReceiver receiver = await client.CreateSessionReceiverAsync("myqueue", sessionId: "mySessionId");
`` You may have noticed that the methods to create a session receiver are async. In order to ensure that a session receiver is usable, it is necessary to first establish a session lock. We establish this lock when calling CreateSessionReceiverAsync, in order to ensure that when the SessionReceiver is returned it will be associated with a particular session that it has the lock for, and that the SessionReceiver properties,SessionIdandSessionLockedUntil` will be populated.

[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

[SeanFeldman]

Consistency
This client also presents an opportunity for making session/non-session usage a bit more consistent as compared to previous libraries. In the previous library, there was a different top-level client when working with sessions, called SessionClient. This client could be used to spawn off receiver instances by calling AcceptSessionMessage.

To accept a session message, wouldn't you require a session to be selected and locked? And if so, it's already an async operation, which the suggested method would have to match by following the Async suffix.

I'm also not clear what is the benefit of this "seamless" consistency. Non-session messages are not correlated. Session messages are. What would be the scenario to Accept a Session message? Unless you the intent is to "Accept (a) Session", but then you've got a duplication with the ServiceBusSessionReceiver described later (under 'Receiving from sessions' section).

Summary: session and non-session messages have very distinct and different scenarios. Merging the two into a single scenario gains what other than API consistency?

Resource usage
By making this sharing be implicit to a ServiceBusClient instance (in fact ServiceBusConnection will no longer be exposed), we can help ensure that applications will not use multiple connections unless they explicitly opt in by creating multiple ServiceBusClient instances.

I.e. connection pooling at the level of the ServiceBusClient. This is what MessagingFactory was doing in Track 0 (yes, that was a thing). This is a better design as it will cause less confusion. Though I'd like to highlight that the objective should not be ensuring fewer connections are established, rather help users understand when a new connection established or not. This approach is safer. 👍

[JoshLove-msft]

To accept a session message, wouldn't you require a session to be selected and locked? And if so, it's already an async operation, which the suggested method would have to match by following the Async suffix.

I had the name of the method wrong. It is called AcceptMessageSessionAsync, as you are accepting a session, not a message. Calling this method is the point at which a session would be selected by the service (if I don't supply a session Id), and the session would be locked.

I'm also not clear what is the benefit of this "seamless" consistency. Non-session messages are not correlated. Session messages are. What would be the scenario to Accept a Session message? Unless you the intent is to "Accept (a) Session", but then you've got a duplication with the ServiceBusSessionReceiver described later (under 'Receiving from sessions' section).

Summary: session and non-session messages have very distinct and different scenarios. Merging the two into a single scenario gains what other than API consistency?

I think the consistency is more about the entry point into the API. If you want to do session receiving without using the pump, in the previous library you would have to start by constructing a SessionClient, which is different than the type you would start with when not using Sessions (i.e. the MessageReceiver).
There are separate types for session vs. non-session receiving in this library as well, but the code written for setting up session vs non-session receiving would be very similar.

[SeanFeldman]

I had the name of the method wrong. It is called AcceptMessageSessionAsync, as you are accepting a session, not a message. Calling this method is the point at which a session would be selected by the service (if I don't supply a session Id), and the session would be locked.

In that case, you still have it off, as the section calls it out as AcceptMessageSession rather than AcceptMessageSessionAsync. The Async suffix is missing. Then it makes sense 👍

What would serviceBusClient.AcceptMessageSessionAsync(...) return?

[JoshLove-msft]

I had the name of the method wrong. It is called AcceptMessageSessionAsync, as you are accepting a session, not a message. Calling this method is the point at which a session would be selected by the service (if I don't supply a session Id), and the session would be locked.

In that case, you still have it off, as the section calls it out as AcceptMessageSession rather than AcceptMessageSessionAsync. The Async suffix is missing. Then it makes sense 👍

What would serviceBusClient.AcceptMessageSessionAsync(...) return?

Updated..
This method is used in the Track 1 library so it will be called off of the sessionClient.
sessionClient.AcceptMessageSessionAsync would return a MessageSessionobject which derives from the MessageReceiver.

[Einarsson]

+1 for the emphasis on approachability. This is also more consistent with other SDKs.

[ShivangiReja]

/cc @danielmarbach

[danielmarbach]

I'm 👍 on what is outlined in the reasoning in the description of this issue. The only thing I was opposed to was to prefix the factory methods with Get and I would have proposed a Create prefix but I see this has already been addressed in the code but hasn't been updated/reflected in the issue description.

[JoshLove-msft]

I'm 👍 on what is outlined in the reasoning in the description of this issue. The only thing I was opposed to was to prefix the factory methods with Get and I would have proposed a Create prefix but I see this has already been addressed in the code but hasn't been updated/reflected in the issue description.

Updated!

[SeanFeldman]

@JoshLove-msft I'm somewhat confused. The methods where already prefixed with Create. Also, the preview package had the right prefixes on the factory method names. So other than the wording, what else has changed?

[JoshLove-msft]

@JoshLove-msft I'm somewhat confused. The methods where already prefixed with Create. Also, the preview package had the right prefixes on the factory method names. So other than the wording, what else has changed?

I meant that I updated the code snippets in this issue to match what we are already using.