Added support in ServiceBusSessionMessageActions for null in GetSessionState and SetSessionState

[Meorir]

Added support for ServiceBusSessionMessageActions:

• SetSessionState(null)
• and returning null from GetSessionState

Issue describing the changes in this PR

resolves #2548

Pull request checklist

• My changes do not require documentation changes
  • Otherwise: Documentation issue linked to PR
• My changes should not be added to the release notes for the next release
  • Otherwise: I've added my notes to release_notes.md
• My changes do not need to be backported to a previous version
  • Otherwise: Backport tracked by issue/PR #issue_or_pr
• I have added all required tests (Unit tests, E2E tests)

[Meorir]

@microsoft-github-policy-service agree

[jviau]

/azp run dotnet-worker.public

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Meorir]

@jviau changed to Moq from manual mock.
Should I also change other test methods in this class to use moq??

[jviau]

Should I also change other test methods in this class to use moq??

Only need to change the tests you added. Thanks!

[JoshLove-msft]

Wouldn't this be considered a build breaking change? Is this something we are allowing for the worker extensions? If so, we should explicitly document this.

[Meorir]

For now, i belive the only way to check for null session state is to
if(binaryDataFromBytes.ToMemory().Length == 0)
if(binaryDataFromBytes.ToArray().Length == 0)

If breaking changes are not to be introduced, maybe it would be better to return BinaryData.Empty rather then BinaryData.FromBytes(memory) in case of empty. Below are my findings.

ReadOnlyMemory<byte> memory = System.Array.Empty<byte>();

BinaryData binaryDataFromBytes = BinaryData.FromBytes(memory);
BinaryData binaryDataFromDataEmpty = BinaryData.Empty;

if (binaryDataFromBytes == BinaryData.Empty) //False
{
	Console.WriteLine("binaryDataFromBytes == BinaryData.Empty");
}

if (binaryDataFromDataEmpty == BinaryData.Empty) //True
{
	Console.WriteLine("binaryDataFromDataEmpty == BinaryData.Empty");
}

if (binaryDataFromBytes.ToMemory().Length == 0) //True
{
	Console.WriteLine("binaryDataFromBytes == ToMemory().Length == 0");
}


if (binaryDataFromDataEmpty.ToMemory().Length == 0) //True
{
	Console.WriteLine("binaryDataFromDataEmpty == ToMemory().Length == 0");
}

if (binaryDataFromBytes.ToArray().Length == 0) //True
{
	Console.WriteLine("binaryDataFromBytes == ToArray().Length == 0");
}


if (binaryDataFromDataEmpty.ToArray().Length == 0) //True
{
	Console.WriteLine("binaryDataFromDataEmpty == ToArray().Length == 0");
}

The documentation (for receiver, not ServiceBusSessionMessageActions) states:
https://learn.microsoft.com/en-us/azure/service-bus-messaging/message-sessions

The methods for managing session state, SetState and GetState, can be found on the session receiver object. A session that had previously no session state returns a null reference for GetState. The previously set session state can be cleared by passing null to the SetState method on the receiver.

[jviau]

Since BinaryData is a class, I not sure if adding ? is a breaking change. I think it is only compiler metadata impacted, not runtime impact. I can't find anything online to confirm that though. @fabiocav do you know?

But if this is a behavior change or departure from in-proc behavior, that I can't comment on.

[Meorir]

In-proc returns null for empty SessionState