Azure-sdk-for-net: [QUERY] ServiceBusReceivedMessage

[SeanFeldman]

Query/Question

1. Is ServiceBusReceivedMessage supposed to be immutable?
2. How a custom testing code that requires ServiceBusReceivedMessage would obtain the object w/o reflection?

Environment:

• Name and version of the Library package used: Azure.Messaging.ServiceBus/7.0.0-preview.2
• Hosting platform or OS and .NET runtime version Windows 10 .NET / Framework 4.8 / Core 3.1
• IDE and version : Visual Studio 2019

[SeanFeldman]

cc @JoshLove-msft

[JoshLove-msft]

Yes, it should be immutable for production purposes. We will need to expose a factory class/method for testing purposes.

[SeanFeldman]

We will need to expose a factory class/method for testing purposes.

Any samples I can follow to find a proper implementation?

[JoshLove-msft]

We will need to expose a factory class/method for testing purposes.

Any samples I can follow to find a proper implementation?

Here is an example from Storage that we would likely use for reference:
https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Blobs/src/Models/BlobsModelFactory.cs

[SeanFeldman]

Yes, it should be immutable for production purposes.

The ServiceBusReceivedMessage.Properties cannot be a IDictionary as it will allow to mutable the collection.

https://github.com/Azure/azure-sdk-for-net/blob/d558a4abda10223148c7a3297c6bf017ba764efb/sdk/servicebus/Azure.Messaging.ServiceBus/src/ServiceBusReceivedMessage.cs#L156

[JoshLove-msft]

Yes, it should be immutable for production purposes.

The ServiceBusReceivedMessage.Properties cannot be a IDictionary as it will allow to mutable the collection.

https://github.com/Azure/azure-sdk-for-net/blob/d558a4abda10223148c7a3297c6bf017ba764efb/sdk/servicebus/Azure.Messaging.ServiceBus/src/ServiceBusReceivedMessage.cs#L156

Good catch - we can probably change to ImmutableDictionary.

[SeanFeldman]

That means AmqpMessageConverter should not be accessing the Property field to assign values.

[SeanFeldman]

@JoshLove-msft System.Collections.Immutable would need to be referenced from the SB project or Packages.Data.props?

[JoshLove-msft]

Both, you will be prevented from referencing a specific version in the csproj file, so the version would go in Packages.Data.props, as I see you have already figured out 😄

[SeanFeldman]

Sent a PR. Most likely will need to be adjusted.

[bjuris]

Took me a while to find the factory, it also had intellisense disabled on the methods, thought I shared a code snippet on this issue if someone else like me googles their way here :smiley:
var msg = ServiceBusModelFactory.ServiceBusReceivedMessage(body: xx)

[SeanFeldman]

I’ve provided the feedback to the team to cover the testing aspect for the track 2 SDK in the upgrade guide. Too bad it’s not linked to or included in the repo’s readme.md.

@jsquire or @jongio, do you think that could be done?

[JoshLove-msft]

Hi @SeanFeldman, https://github.com/Azure/azure-sdk-for-net/issues/16788 is something we will be working on this month. This will include adding information about the model factories to the migration guide.

For reference, the README does link to the migration guide - https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/servicebus/Azure.Messaging.ServiceBus#azure-service-bus-client-library-for-net (see the link at the bottom of this section)

[SeanFeldman]

Thank you, @JoshLove-msft.

For reference, the README does link to the migration guide - master/sdk/servicebus/Azure.Messaging.ServiceBus#azure-service-bus-client-library-for-net (see the link at the bottom of this section)

It does but the migration guide doesn't have anything on testing and model factories.

[JoshLove-msft]

Thank you, @JoshLove-msft.

For reference, the README does link to the migration guide - master/sdk/servicebus/Azure.Messaging.ServiceBus#azure-service-bus-client-library-for-net (see the link at the bottom of this section)

It does but the migration guide doesn't have anything on testing and model factories.

Yep, that is what we will be working on this month - cleaning up the migration guide and adding missing details such as the model factories.

[jsquire]

We should also consider putting out samples focused on testing and illustrating mocking for major types. I've opened #17380 to track that.