MessageSender abstraction is broken for Path when Send-Via is used

[SeanFeldman]

Actual Behavior

MessageSender supports Send-Via feature. Send-Via is using a transfer queue to route message to their final destination when using transactional processing. The abstraction is represented by a public overload of MessageSender that takes in two paths

1. entitytPath
2. viaEntityPath
   https://github.com/Azure/azure-service-bus-dotnet/blob/e278f87b0b3aa563ab055918e988baec9ec155a3/src/Microsoft.Azure.ServiceBus/Core/MessageSender.cs#L127-L135

The abstraction has been copied from the old client and works, but it's broken when Path and TransferDestinationPath are queried back. That's because all public overloads are invoking an internal overload, swapping the two paths:
https://github.com/Azure/azure-service-bus-dotnet/blob/e278f87b0b3aa563ab055918e988baec9ec155a3/src/Microsoft.Azure.ServiceBus/Core/MessageSender.cs#L132

where internal overload is
https://github.com/Azure/azure-service-bus-dotnet/blob/e278f87b0b3aa563ab055918e988baec9ec155a3/src/Microsoft.Azure.ServiceBus/Core/MessageSender.cs#L137-L144

assigning viaEntityPath to this.Path and entityPath to this.TransferDestinationPath. Which is inverted.

Expected Behavior

Path and Send-Via/TransferDestinationPath after MessageSender constructor is invoked should be identical to the values passed in.

Repro

LinqPad repro script attached.

var sender1 = new MessageSender(connection, "path");
sender1.Path.Dump("Should be 'path'");
Assert.IsNull(sender1.TransferDestinationPath); // green

var sender2 = new MessageSender(connection, "path", "intermediate");
Assert.AreEqual("path", sender2.Path); // red, equals to "intermediate"
Assert.AreEqual("intermediate", sender2.TransferDestinationPath); // red, equals to "path"

Recommendations

The fix should be released as a major version for the following reasons:

1. This is a breaking change. While a fix will restore the correct behaviour, it will affect any existing solution relying on the incorrect implementation.
2. Anyone implementing a workaround will be broken if they update to a patch/minor.

[SeanFeldman]

Added a validation PR against master to confirm this behaviour

[binzywu]

Thank you @SeanFeldman for reporting this.

[SeanFeldman]

@nemakam this will be handled in 4.0.0?

[nemakam]

Yes

[SeanFeldman]

@jsquire @AlexGhiondea have you had a chance to triage this one? This is a client bug.

[AlexGhiondea]

@SeanFeldman we will consider this for the next release of the Service Bus client. We currently do not have a release date for when that is going to be.

[SeanFeldman]

@AlexGhiondea note that this will be a breaking change and will require a new major.
In that case, you might want to introduce other breaking changes, such as #6037.

[SeanFeldman]

@AlexGhiondea any updates about next (major) release and when this will be addressed?

[SeanFeldman]

@AlexGhiondea @jsquire would be nice to label this issue as bug to indicate what it is. Thank you.

[AlexGhiondea]

/cc @Binzy @nemakam

[nemakam]

Closed in #6941

[SeanFeldman]

Needs to be a minor with the change made in PR.
#6941 (comment)