WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Added support in ServiceBusSessionMessageActions for null in GetSessionState and SetSessionState #2912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Meorir wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Added support in ServiceBusSessionMessageActions for null in GetSessionState and SetSessionState #2912

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94d06dd
Added support for null in
Jan 4, 2025
1e4b979
updated release notes
Jan 6, 2025
f1a5e61
Changed to Moq from manual mock
Jan 9, 2025
87132df
Redesigned tests
Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion extensions/Worker.Extensions.ServiceBus/release_notes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@
### Microsoft.Azure.Functions.Worker.Extensions.ServiceBus 5.22.0

- Updated `Azure.Identity` reference to 1.12.0
- Updated `Microsoft.Extensions.Azure` to 1.7.5
- Updated `Microsoft.Extensions.Azure` to 1.7.5
- Added 'null' support in SetSessionState and GetSessionState methods (#2548)
16 changes: 11 additions & 5 deletions extensions/Worker.Extensions.ServiceBus/src/ServiceBusSessionMessageActions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected ServiceBusSessionMessageActions()
public virtual DateTimeOffset SessionLockedUntil { get; protected set; }

///<inheritdoc cref="ServiceBusReceiver.CompleteMessageAsync(ServiceBusReceivedMessage, CancellationToken)"/>
public virtual async Task<BinaryData> GetSessionStateAsync(
public virtual async Task<BinaryData?> GetSessionStateAsync(
Copy link
Member

@JoshLove-msft JoshLove-msft Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@Meorir Meorir Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@jviau jviau Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@Meorir Meorir Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In-proc returns null for empty SessionState

CancellationToken cancellationToken = default)
{
var request = new GetSessionStateRequest()
Expand All @@ -52,19 +52,25 @@ public virtual async Task<BinaryData> GetSessionStateAsync(
};

GetSessionStateResponse result = await _settlement.GetSessionStateAsync(request, cancellationToken: cancellationToken);
BinaryData binaryData = new BinaryData(result.SessionState.Memory);
return binaryData;

if (result.SessionState is null || result.SessionState.IsEmpty)
{
return null;
}

return new BinaryData(result.SessionState.Memory);
}

///<inheritdoc cref="ServiceBusReceiver.CompleteMessageAsync(ServiceBusReceivedMessage, CancellationToken)"/>
public virtual async Task SetSessionStateAsync(
BinaryData sessionState,
BinaryData? sessionState,
CancellationToken cancellationToken = default)

{
var request = new SetSessionStateRequest()
{
SessionId = _sessionId,
SessionState = ByteString.CopyFrom(sessionState.ToMemory().Span),
SessionState = sessionState is null ? ByteString.Empty : ByteString.CopyFrom(sessionState.ToMemory().Span),
};

await _settlement.SetSessionStateAsync(request, cancellationToken: cancellationToken);
Expand Down
50 changes: 50 additions & 0 deletions test/Worker.Extensions.Tests/ServiceBus/ServiceBusSessionMessageActionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Google.Protobuf.WellKnownTypes;
using Grpc.Core;
using Microsoft.Azure.ServiceBus.Grpc;
using Moq;
using Xunit;

namespace Microsoft.Azure.Functions.Worker.Extensions.Tests
Expand Down Expand Up @@ -49,6 +50,55 @@ public async Task CanRenewSessionLock()
await messageActions.RenewSessionLockAsync();
}

[Fact]
public async Task CanSetAndGetSessionStateWithNull()
{
var sessionId = "test";
var initialTestString = "TestInitialState";
ByteString currentSessionState = ByteString.CopyFromUtf8(initialTestString);

var mockClient = new Mock<Settlement.SettlementClient>();

mockClient
.Setup(x => x.GetSessionStateAsync(
It.Is<GetSessionStateRequest>(r => r.SessionId == sessionId),
It.IsAny<Metadata>(),
It.IsAny<DateTime?>(),
It.IsAny<CancellationToken>())
)
.Returns(() => CreateAsyncUnaryCall(new GetSessionStateResponse
{
SessionState = currentSessionState
}));

mockClient
.Setup(x => x.SetSessionStateAsync(
It.Is<SetSessionStateRequest>(r => r.SessionId == sessionId),
It.IsAny<Metadata>(),
It.IsAny<DateTime?>(),
It.IsAny<CancellationToken>())
)
.Returns<SetSessionStateRequest, Metadata, DateTime?, CancellationToken>((request, _, _, _) =>
{
currentSessionState = request.SessionState;
return CreateAsyncUnaryCall(new Empty());
});

var message = ServiceBusModelFactory.ServiceBusReceivedMessage(lockTokenGuid: Guid.NewGuid(), sessionId: sessionId);
var messageActions = new ServiceBusSessionMessageActions(settlement: mockClient.Object,sessionId: message.SessionId,sessionLockedUntil: message.LockedUntil);

var initialState = await messageActions.GetSessionStateAsync();
Assert.Equal(initialTestString, initialState.ToString());

await messageActions.SetSessionStateAsync(null);
var afterSetNullState = await messageActions.GetSessionStateAsync();
Assert.Null(afterSetNullState);
}
private static AsyncUnaryCall<T> CreateAsyncUnaryCall<T>(T response) where T : class
{
return new AsyncUnaryCall<T>(Task.FromResult(response),Task.FromResult(new Metadata()),() => Status.DefaultSuccess,() => new Metadata(),() => { });
}

private class MockSettlementClient : Settlement.SettlementClient
{
private readonly string _sessionId;
Expand Down